MDL-58267 completion: style and capabilities cleanup

Part of MDL-58138 epic

diff --git a/completion/classes/bulkedit_form.php b/completion/classes/bulkedit_form.php
index 7e92918..2d14f3f 100644 (file)
--- a/completion/classes/bulkedit_form.php
+++ b/completion/classes/bulkedit_form.php
@@ -53,6 +53,47 @@ class core_completion_bulkedit_form extends core_completion_edit_base_form {
         return $this->_modnames;
     }
 
+    /**
+     * Returns an instance of component-specific module form for the first selected module
+     *
+     * @return moodleform_mod|null
+     */
+    protected function get_module_form() {
+        global $CFG, $PAGE;
+
+        if ($this->_moduleform) {
+            return $this->_moduleform;
+        }
+
+        $cm = reset($this->cms);
+        $course = $this->course;
+        $modname = $cm->modname;
+
+        $modmoodleform = "$CFG->dirroot/mod/$modname/mod_form.php";
+        if (file_exists($modmoodleform)) {
+            require_once($modmoodleform);
+        } else {
+            print_error('noformdesc');
+        }
+
+        list($cmrec, $context, $module, $data, $cw) = get_moduleinfo_data($cm, $course);
+        $data->return = 0;
+        $data->sr = 0;
+        $data->update = $modname;
+
+        // Initialise the form but discard all JS requirements it adds, our form has already added them.
+        $mformclassname = 'mod_'.$modname.'_mod_form';
+        if (!defined('PHPUNIT_TEST') || !PHPUNIT_TEST) {
+            $PAGE->start_collecting_javascript_requirements();
+        }
+        $this->_moduleform = new $mformclassname($data, 0, $cmrec, $course);
+        if (!defined('PHPUNIT_TEST') || !PHPUNIT_TEST) {
+            $PAGE->end_collecting_javascript_requirements();
+        }
+
+        return $this->_moduleform;
+    }
+
     /**
      * Form definition
      */

diff --git a/completion/classes/defaultedit_form.php b/completion/classes/defaultedit_form.php
index 7a7b554..f370c2a 100644 (file)
--- a/completion/classes/defaultedit_form.php
+++ b/completion/classes/defaultedit_form.php
@@ -53,6 +53,47 @@ class core_completion_defaultedit_form extends core_completion_edit_base_form {
         return $this->_modnames;
     }
 
+    /**
+     * Returns an instance of component-specific module form for the first selected module
+     *
+     * @return moodleform_mod|null
+     */
+    protected function get_module_form() {
+        global $CFG, $PAGE;
+
+        if ($this->_moduleform) {
+            return $this->_moduleform;
+        }
+
+        $modnames = array_keys($this->get_module_names());
+        $modname = $modnames[0];
+        $course = $this->course;
+
+        $modmoodleform = "$CFG->dirroot/mod/$modname/mod_form.php";
+        if (file_exists($modmoodleform)) {
+            require_once($modmoodleform);
+        } else {
+            print_error('noformdesc');
+        }
+
+        list($module, $context, $cw, $cmrec, $data) = prepare_new_moduleinfo_data($course, $modname, 0);
+        $data->return = 0;
+        $data->sr = 0;
+        $data->add = $modname;
+
+        // Initialise the form but discard all JS requirements it adds, our form has already added them.
+        $mformclassname = 'mod_'.$modname.'_mod_form';
+        if (!defined('PHPUNIT_TEST') || !PHPUNIT_TEST) {
+            $PAGE->start_collecting_javascript_requirements();
+        }
+        $this->_moduleform = new $mformclassname($data, 0, $cmrec, $course);
+        if (!defined('PHPUNIT_TEST') || !PHPUNIT_TEST) {
+            $PAGE->end_collecting_javascript_requirements();
+        }
+
+        return $this->_moduleform;
+    }
+
     /**
      * Form definition,
      */

diff --git a/completion/classes/edit_base_form.php b/completion/classes/edit_base_form.php
index 34e29dd..676d712 100644 (file)
--- a/completion/classes/edit_base_form.php
+++ b/completion/classes/edit_base_form.php
@@ -83,41 +83,7 @@ abstract class core_completion_edit_base_form extends moodleform {
      *
      * @return moodleform_mod|null
      */
-    protected function get_module_form() {
-        global $CFG, $PAGE;
-
-        if ($this->_moduleform) {
-            return $this->_moduleform;
-        }
-
-        $modnames = array_keys($this->get_module_names());
-        $modname = $modnames[0];
-        $course = $this->course;
-
-        $modmoodleform = "$CFG->dirroot/mod/$modname/mod_form.php";
-        if (file_exists($modmoodleform)) {
-            require_once($modmoodleform);
-        } else {
-            print_error('noformdesc');
-        }
-
-        list($module, $context, $cw, $cmrec, $data) = prepare_new_moduleinfo_data($course, $modname, 0);
-        $data->return = 0;
-        $data->sr = 0;
-        $data->add = $modname;
-
-        // Initialise the form but discard all JS requirements it adds, our form has already added them.
-        $mformclassname = 'mod_'.$modname.'_mod_form';
-        if (!defined('PHPUNIT_TEST') || !PHPUNIT_TEST) {
-            $PAGE->start_collecting_javascript_requirements();
-        }
-        $this->_moduleform = new $mformclassname($data, 0, $cmrec, $course);
-        if (!defined('PHPUNIT_TEST') || !PHPUNIT_TEST) {
-            $PAGE->end_collecting_javascript_requirements();
-        }
-
-        return $this->_moduleform;
-    }
+    abstract protected function get_module_form();
 
     /**
      * If all selected modules are of the same module type, adds custom completion rules from this module type

diff --git a/completion/classes/manager.php b/completion/classes/manager.php
index 20c43d4..fd785ac 100644 (file)
--- a/completion/classes/manager.php
+++ b/completion/classes/manager.php
@@ -29,6 +29,9 @@ namespace core_completion;
 use stdClass;
 use context_course;
 use cm_info;
+use tabobject;
+use lang_string;
+use moodle_url;
 
 /**
  * Bulk activity completion manager class
@@ -234,6 +237,43 @@ class manager {
         return false;
     }
 
+    /**
+     * @param stdClass|int $courseorid
+     * @return tabobject[]
+     */
+    public static function get_available_completion_tabs($courseorid) {
+        $tabs = [];
+
+        $courseid = is_object($courseorid) ? $courseorid->id : $courseorid;
+        $coursecontext = context_course::instance($courseid);
+
+        if (has_capability('moodle/course:update', $coursecontext)) {
+            $tabs[] = new tabobject(
+                'completion',
+                new moodle_url('/course/completion.php', ['id' => $courseid]),
+                new lang_string('coursecompletion', 'completion')
+            );
+        }
+
+        if (has_capability('moodle/course:manageactivities', $coursecontext)) {
+            $tabs[] = new tabobject(
+                'defaultcompletion',
+                new moodle_url('/course/defaultcompletion.php', ['id' => $courseid]),
+                new lang_string('defaultcompletion', 'completion')
+            );
+        }
+
+        if (self::can_edit_bulk_completion($courseorid)) {
+            $tabs[] = new tabobject(
+                'bulkcompletion',
+                new moodle_url('/course/bulkcompletion.php', ['id' => $courseid]),
+                new lang_string('bulkactivitycompletion', 'completion')
+            );
+        }
+
+        return $tabs;
+    }
+
     /**
      * Applies completion from the bulk edit form to all selected modules
      *

diff --git a/course/bulkcompletion.php b/course/bulkcompletion.php
index 7f1c560..900cb71 100644 (file)
--- a/course/bulkcompletion.php
+++ b/course/bulkcompletion.php
@@ -57,12 +57,12 @@ $PAGE->set_title($course->shortname);
 $PAGE->set_heading($course->fullname);
 $PAGE->set_pagelayout('admin');
 
-// Get all that stuff I need for the renderer.
+// Check access.
 if (!core_completion\manager::can_edit_bulk_completion($id)) {
-    throw new required_capability_exception(context_course::instance($course->id),
-        'moodle/course:manageactivities', 'nopermission');
+    require_capability('moodle/course:manageactivities', context_course::instance($course->id));
 }
 
+// Get all that stuff I need for the renderer.
 $manager = new \core_completion\manager($id);
 $bulkcompletiondata = $manager->get_activities_and_headings();
 
@@ -72,7 +72,7 @@ $renderer = $PAGE->get_renderer('core_course', 'bulk_activity_completion');
 echo $OUTPUT->header();
 echo $OUTPUT->heading(get_string('bulkactivitycompletion', 'completion'));
 
-echo $renderer->navigation($id, 'bulkcompletion');
+echo $renderer->navigation($course, 'bulkcompletion');
 
 $PAGE->requires->yui_module('moodle-core-formchangechecker',
         'M.core_formchangechecker.init',

diff --git a/course/classes/output/bulk_activity_completion_renderer.php b/course/classes/output/bulk_activity_completion_renderer.php
index e35eb88..0a746e9 100644 (file)
--- a/course/classes/output/bulk_activity_completion_renderer.php
+++ b/course/classes/output/bulk_activity_completion_renderer.php
@@ -35,31 +35,8 @@ require_once($CFG->dirroot.'/course/renderer.php');
  */
 class core_course_bulk_activity_completion_renderer extends plugin_renderer_base {
 
-    public function navigation($courseid, $page) {
-        $tabs = [];
-
-        if (has_capability('moodle/course:update', context_course::instance($courseid))) {
-            $tabs[] = new tabobject(
-                'completion',
-                new moodle_url('/course/completion.php', ['id' => $courseid]),
-                get_string('coursecompletion', 'completion')
-            );
-
-            $tabs[] = new tabobject(
-                'defaultcompletion',
-                new moodle_url('/course/defaultcompletion.php', ['id' => $courseid]),
-                get_string('defaultcompletion', 'completion')
-            );
-        }
-
-        if (core_completion\manager::can_edit_bulk_completion($courseid)) {
-            $tabs[] = new tabobject(
-                'bulkcompletion',
-                new moodle_url('/course/bulkcompletion.php', ['id' => $courseid]),
-                get_string('bulkactivitycompletion', 'completion')
-            );
-        }
-
+    public function navigation($courseorid, $page) {
+        $tabs = core_completion\manager::get_available_completion_tabs($courseorid);
         if (count($tabs) > 1) {
             return $this->tabtree($tabs, $page);
         } else {

diff --git a/course/completion.php b/course/completion.php
index c1967a8..26f03cf 100644 (file)
--- a/course/completion.php
+++ b/course/completion.php
@@ -55,8 +55,10 @@ if ($id) {
     require_login($course);
     $context = context_course::instance($course->id);
     if (!has_capability('moodle/course:update', $context)) {
-        if (core_completion\manager::can_edit_bulk_completion($course)) {
-            redirect(new moodle_url('/course/bulkcompletion.php', ['id' => $course->id]));
+        // User is not allowed to modify course completion.
+        // Check if they can see default completion or edit bulk completion and redirect there.
+        if ($tabs = core_completion\manager::get_available_completion_tabs($course)) {
+            redirect($tabs[0]->link);
         } else {
             require_capability('moodle/course:update', $context);
         }
@@ -161,7 +163,7 @@ $renderer = $PAGE->get_renderer('core_course', 'bulk_activity_completion');
 echo $OUTPUT->header();
 echo $OUTPUT->heading(get_string('editcoursecompletionsettings', 'core_completion'));
 
-echo $renderer->navigation($id, 'completion');
+echo $renderer->navigation($course, 'completion');
 
 $form->display();
 

diff --git a/course/defaultcompletion.php b/course/defaultcompletion.php
index f02f1b8..3f45696 100644 (file)
--- a/course/defaultcompletion.php
+++ b/course/defaultcompletion.php
@@ -42,7 +42,7 @@ if ($id) {
         print_error('invalidcourseid');
     }
     require_login($course);
-    require_capability('moodle/course:update', context_course::instance($course->id));
+    require_capability('moodle/course:manageactivities', context_course::instance($course->id));
 
 } else {
     require_login();
@@ -67,7 +67,7 @@ $renderer = $PAGE->get_renderer('core_course', 'bulk_activity_completion');
 echo $OUTPUT->header();
 echo $OUTPUT->heading(get_string('defaultcompletion', 'completion'));
 
-echo $renderer->navigation($id, 'defaultcompletion');
+echo $renderer->navigation($course, 'defaultcompletion');
 
 $PAGE->requires->yui_module('moodle-core-formchangechecker',
         'M.core_formchangechecker.init',

diff --git a/course/editbulkcompletion.php b/course/editbulkcompletion.php
index 728565c..65d60ac 100644 (file)
--- a/course/editbulkcompletion.php
+++ b/course/editbulkcompletion.php
@@ -37,8 +37,7 @@ $PAGE->set_heading($course->fullname);
 $PAGE->set_pagelayout('admin');
 
 if (!core_completion\manager::can_edit_bulk_completion($course)) {
-    throw new required_capability_exception(context_course::instance($course->id),
-        'moodle/course:manageactivities', 'nopermission');
+    require_capability('moodle/course:manageactivities', context_course::instance($course->id));
 }
 
 // Prepare list of modules to be updated.
@@ -70,7 +69,7 @@ $renderer = $PAGE->get_renderer('core_course', 'bulk_activity_completion');
 echo $OUTPUT->header();
 echo $OUTPUT->heading(get_string('bulkactivitycompletion', 'completion'));
 
-echo $renderer->navigation($course->id, 'bulkcompletion');
+echo $renderer->navigation($course, 'bulkcompletion');
 
 echo $renderer->edit_bulk_completion($form, $manager->get_activities(array_keys($cms)));
 

diff --git a/course/editdefaultcompletion.php b/course/editdefaultcompletion.php
index 7f29bbd..17897e7 100644 (file)
--- a/course/editdefaultcompletion.php
+++ b/course/editdefaultcompletion.php
@@ -67,7 +67,7 @@ $renderer = $PAGE->get_renderer('core_course', 'bulk_activity_completion');
 echo $OUTPUT->header();
 echo $OUTPUT->heading(get_string('defaultcompletion', 'completion'));
 
-echo $renderer->navigation($course->id, 'defaultcompletion');
+echo $renderer->navigation($course, 'defaultcompletion');
 
 echo $renderer->edit_default_completion($form, $modules);
 

diff --git a/course/lib.php b/course/lib.php
index 88996b8..6006ded 100644 (file)
--- a/course/lib.php
+++ b/course/lib.php
@@ -3870,7 +3870,8 @@ function course_get_user_administration_options($course, $context) {
 
     $options = new stdClass;
     $options->update = has_capability('moodle/course:update', $context);
-    $options->editcompletion = core_completion\manager::can_edit_bulk_completion($course);
+    $options->editcompletion = $CFG->enablecompletion && $course->enablecompletion &&
+        ($options->update || count(core_completion\manager::get_available_completion_tabs($course, $context)) > 0);
     $options->filters = has_capability('moodle/filter:manage', $context) &&
                         count(filter_get_available_in_context($context)) > 0;
     $options->reports = has_capability('moodle/site:viewreports', $context);

diff --git a/course/templates/activityinstance.mustache b/course/templates/activityinstance.mustache
index 878dbfd..3843a8c 100644 (file)
--- a/course/templates/activityinstance.mustache
+++ b/course/templates/activityinstance.mustache
@@ -49,7 +49,7 @@
     <div class="activity-completionstatus col-sm-6">
         <div class="col-sm-1">
             {{#completionstatus.icon}}
-                <img src="{{completionstatus.icon}}" class="m-r-2">
+                <img src="{{completionstatus.icon}}" class="m-r-2" alt=" " role="presentation">
             {{/completionstatus.icon}}
             {{^completionstatus.icon}}
                 <span class="m-r-3"></span>

diff --git a/course/templates/bulkactivitycompletion.mustache b/course/templates/bulkactivitycompletion.mustache
index 33937a0..78cae04 100644 (file)
--- a/course/templates/bulkactivitycompletion.mustache
+++ b/course/templates/bulkactivitycompletion.mustache
@@ -60,9 +60,9 @@
         </div>
     </div>
     <div class="topics">
-        
+
         {{#sections}}
-        
+
                 <div class="topic-section m-b-1">
                     <div class="row m-b-1">
                         <div class="col-sm-12">
@@ -72,9 +72,9 @@
                     </div>
                     {{> core_course/activityinstance}}
                 </div>
-        
+
         {{/sections}}
-        
+
     </div>
     <input type="hidden" name="id" value="{{courseid}}" />
     <input type="hidden" name="sesskey" value="{{sesskey}}" />

diff --git a/course/templates/defaultactivitycompletion.mustache b/course/templates/defaultactivitycompletion.mustache
index d4985ba..2c1346b 100644 (file)
--- a/course/templates/defaultactivitycompletion.mustache
+++ b/course/templates/defaultactivitycompletion.mustache
@@ -61,15 +61,16 @@
             {{#canmanage}}
              <div class="module-section m-b-1">
                 <div class="row m-b-1">
-                    <div class="col-sm-12">
-                        <input type="checkbox" class="m-r-1" name="modids[]" value="{{id}}" aria-label="{{#str}}checkactivity, completion, {{formatedname}}{{/str}}">
-                        <img src={{icon}} />
+                    <div class="col-sm-6">
+                        <label class="accesshide" for="modtype_{{id}}">Select {{formattedname}}</label>
+                        <input id="modtype_{{id}}" type="checkbox" class="m-r-1" name="modids[]" value="{{id}}" aria-label="{{#str}}checkactivity, completion, {{formattedname}}{{/str}}">
+                        <img src="{{icon}}" alt=" " role="presentation" />
                         <span>{{formattedname}}</span>
                     </div>
                     <div class="activity-completionstatus col-sm-6">
                         <div class="col-sm-1">
                             {{#completionstatus.icon}}
-                                <img src="{{completionstatus.icon}}" class="m-r-2">
+                                <img src="{{completionstatus.icon}}" class="m-r-2" alt=" " role="presentation">
                             {{/completionstatus.icon}}
                             {{^completionstatus.icon}}
                                 <span class="m-r-3"></span>

diff --git a/course/templates/editdefaultcompletion.mustache b/course/templates/editdefaultcompletion.mustache
index e5e2d25..706761b 100644 (file)
--- a/course/templates/editdefaultcompletion.mustache
+++ b/course/templates/editdefaultcompletion.mustache
@@ -38,7 +38,7 @@
         <div class="row m-b-1">
             {{#modules}}
                 <div class="col-sm-2">
-                    <img src="{{icon}}" class="m-r-1 m-b-1" alt=" " />
+                    <img src="{{icon}}" class="m-r-1 m-b-1" alt=" " role="presentation" />
                     <span>{{{formattedname}}}</span>
                 </div>
             {{/modules}}

diff --git a/lib/navigationlib.php b/lib/navigationlib.php
index f607bf9..c21d22a 100644 (file)
--- a/lib/navigationlib.php
+++ b/lib/navigationlib.php
@@ -4207,13 +4207,10 @@ class settings_navigation extends navigation_node {
             $coursenode->add($editstring, $editurl, self::TYPE_SETTING, null, 'turneditingonoff', new pix_icon('i/edit', ''));
         }
 
-        if ($adminoptions->update || $adminoptions->editcompletion) {
-
+        if ($adminoptions->editcompletion) {
             // Add the course completion settings link
-            if ($CFG->enablecompletion && $course->enablecompletion) {
-                $url = new moodle_url('/course/completion.php', array('id'=>$course->id));
-                $coursenode->add(get_string('coursecompletion', 'completion'), $url, self::TYPE_SETTING, null, null, new pix_icon('i/settings', ''));
-            }
+            $url = new moodle_url('/course/completion.php', array('id' => $course->id));
+            $coursenode->add(get_string('coursecompletion', 'completion'), $url, self::TYPE_SETTING, null, null, new pix_icon('i/settings', ''));
         }
 
         if (!$adminoptions->update && $adminoptions->tags) {