MDL-42190 workshop: Improve the confirmation of submission removal

I noticed we do not need to list the names of potentially affected reviewers. If the user (teacher) has the capability to see other assessments (which they do in 99%), the list of assessments is displayed at the confirmation page anyway. If for some reason they do not have the capability, we should not list the names. The second fix is that the action buttons (such as Delete or Edit) are not displayed on the removal confirmation page. And finally, as the output->confirm() wraps the message with implicit <p>, it is not valid to pass HTML with any block elements. So instead of concatenating the two messages, we simply define a new string for this situation.
moodle · Jan 25, 2016 · 58489e1 · 58489e1
1 parent 3fa2289
commit 58489e1
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 34 deletions.
diff --git a/mod/workshop/lang/en/workshop.php b/mod/workshop/lang/en/workshop.php
@@ -258,7 +258,7 @@
 $string['submissionby'] = 'Submission by {$a}';
 $string['submissioncontent'] = 'Submission content';
 $string['submissiondeleteconfirm'] = 'Are you sure you want to delete the following submission?';
-$string['submissiondeleteconfirmteacher'] = 'This will also delete any assessments associated with this submission, which may affect the following students\' grades:';
+$string['submissiondeleteconfirmassess'] = 'Are you sure you want to delete the following submission? Note this will also delete {$a->count} assessments associated with this submission, which may affect the reviewers\' grades.';
 $string['submissionend'] = 'Submissions deadline';
 $string['submissionendbeforestart'] = 'Submissions deadline can not be specified before the open for submissions date';
 $string['submissionendevent'] = '{$a} (submissions deadline)';

diff --git a/mod/workshop/submission.php b/mod/workshop/submission.php
@@ -351,25 +351,12 @@
 if ($deletable and $delete) {
     $prompt = get_string('submissiondeleteconfirm', 'workshop');
     if ($candeleteall) {
-        $assessments = $workshop->get_assessments_of_submission($submission->id);
-        if (count($assessments) > 0) {
-            $prompt = html_writer::tag('p', $prompt);
-            $prompt .= html_writer::tag('p', get_string('submissiondeleteconfirmteacher', 'workshop'));
-            $affected = '';
-            $fields = get_all_user_name_fields(true);
-            $reviewers = array();
-            foreach ($assessments as $assessment) {
-                if (!in_array($assessment->reviewerid, $reviewers)) {
-                    $reviewers[] = $assessment->reviewerid;
-                    $names = $DB->get_record('user', array('id' => $assessment->reviewerid), $fields);
-                    $affected .= html_writer::tag('li', fullname($names));
-                }
-            }
-            $prompt .= html_writer::tag('ul', $affected);
+        $count = count($workshop->get_assessments_of_submission($submission->id));
+        if ($count > 0) {
+            $prompt = get_string('submissiondeleteconfirmassess', 'workshop', ['count' => $count]);
         }
     }
-    echo $output->confirm($prompt,
-            new moodle_url($PAGE->url, array('delete' => 1, 'confirm' => 1)), $workshop->view_url());
+    echo $output->confirm($prompt, new moodle_url($PAGE->url, ['delete' => 1, 'confirm' => 1]), $workshop->view_url());
 }
 
 // else display the submission
@@ -385,25 +372,28 @@
     echo $output->box(get_string('noyoursubmission', 'workshop'));
 }
 
-if ($editable) {
-    if ($submission->id) {
-        $btnurl = new moodle_url($PAGE->url, array('edit' => 'on', 'id' => $submission->id));
-        $btntxt = get_string('editsubmission', 'workshop');
-    } else {
-        $btnurl = new moodle_url($PAGE->url, array('edit' => 'on'));
-        $btntxt = get_string('createsubmission', 'workshop');
+// If not at removal confirmation screen, some action buttons can be displayed.
+if (!$delete) {
+    if ($editable) {
+        if ($submission->id) {
+            $btnurl = new moodle_url($PAGE->url, array('edit' => 'on', 'id' => $submission->id));
+            $btntxt = get_string('editsubmission', 'workshop');
+        } else {
+            $btnurl = new moodle_url($PAGE->url, array('edit' => 'on'));
+            $btntxt = get_string('createsubmission', 'workshop');
+        }
+        echo $output->single_button($btnurl, $btntxt, 'get');
     }
-    echo $output->single_button($btnurl, $btntxt, 'get');
-}
 
-if ($submission->id and $deletable) {
-    $url = new moodle_url($PAGE->url, array('delete' => 1));
-    echo $output->single_button($url, get_string('deletesubmission', 'workshop'), 'get');
-}
+    if ($submission->id and $deletable) {
+        $url = new moodle_url($PAGE->url, array('delete' => 1));
+        echo $output->single_button($url, get_string('deletesubmission', 'workshop'), 'get');
+    }
 
-if ($submission->id and !$edit and !$isreviewer and $canallocate and $workshop->assessing_allowed($USER->id)) {
-    $url = new moodle_url($PAGE->url, array('assess' => 1));
-    echo $output->single_button($url, get_string('assess', 'workshop'), 'post');
+    if ($submission->id and !$edit and !$isreviewer and $canallocate and $workshop->assessing_allowed($USER->id)) {
+        $url = new moodle_url($PAGE->url, array('assess' => 1));
+        echo $output->single_button($url, get_string('assess', 'workshop'), 'post');
+    }
 }
 
 if (($workshop->phase == workshop::PHASE_CLOSED) and ($ownsubmission or $canviewall)) {