Skip to content
Browse files

MDL-37602 Clean up and improve the overall feedback rendering

In order to use the overall feedback in assessments of example
submissions and in the assessment form preview, significant improvements
in the rendering machinery were done.

Workshop class provides two new methods overall_feedback_content_options()
and overall_feedback_attachment_options() as they are needed at various
scripts and libraries.

Overall feedback is displayed as a part of the workshop_assessment_form
only if the form is in editable mode (not frozen). If the form is
displayed in read-only (frozen) mode, the caller is expected to render
the overall feedback and list of attachments (the editor and filemanager
elements do not support frozen mode). To do so, the renderable
workshop_assessment now loads overall feedback data and provides two new
methods get_overall_feedback_content() and
get_overall_feedback_attachments() to be used by the renderer.

Renderable workshop_submission, workshop_assessment and related classes
now accept the workshop instance as the first parameter in their
constructors. This way, these renderable classes have access to the
workshop API.

In the future, the rendering of submission files should be improved in
the same way as is done in this patch (i.e. moving the logic and data
preparation out of the renderer into the renderable classes).
  • Loading branch information...
1 parent 88e79d8 commit 77d746a2f51c9a5cd2b5e593698654e7cd57904b @mudrd8mz mudrd8mz committed Mar 26, 2013
View
66 mod/workshop/assessment.php
@@ -37,7 +37,6 @@
require_once(dirname(dirname(dirname(__FILE__))).'/config.php');
require_once(dirname(__FILE__).'/locallib.php');
-require_once($CFG->libdir.'/filelib.php');
$asid = required_param('asid', PARAM_INT); // assessment id
$assessment = $DB->get_record('workshop_assessments', array('id' => $asid), '*', MUST_EXIST);
@@ -136,66 +135,23 @@
$pending = array();
}
// load the assessment form and process the submitted data eventually
- $feedbackauthoreditoropts = array(
- 'maxbytes' => $workshop->overallfeedbackmaxbytes,
- 'maxfiles' => $workshop->overallfeedbackfiles,
- 'changeformat' => 1,
- 'context' => $workshop->context,
- );
- $feedbackauthorattachmentopts = array(
- 'maxbytes' => $workshop->overallfeedbackmaxbytes,
- 'maxfiles' => $workshop->overallfeedbackfiles,
- 'return_types' => FILE_INTERNAL,
- );
- $mform = $strategy->get_assessment_form($PAGE->url, 'assessment', $assessment, $assessmenteditable, array(
- 'editableweight' => $cansetassessmentweight,
- 'pending' => !empty($pending),
- 'feedbackauthoreditoropts' => $feedbackauthoreditoropts,
- 'feedbackauthorattachmentopts' => $feedbackauthorattachmentopts,
- ));
+ $mform = $strategy->get_assessment_form($PAGE->url, 'assessment', $assessment, $assessmenteditable,
+ array('editableweight' => $cansetassessmentweight, 'pending' => !empty($pending)));
// Set data managed by the workshop core, subplugins set their own data themselves.
$currentdata = (object)array(
'weight' => $assessment->weight,
'feedbackauthor' => $assessment->feedbackauthor,
'feedbackauthorformat' => $assessment->feedbackauthorformat,
);
- if (!empty($workshop->overallfeedbackmode)) {
- if ($assessmenteditable) {
- $currentdata = file_prepare_standard_editor($currentdata, 'feedbackauthor', $feedbackauthoreditoropts,
- $workshop->context, 'mod_workshop', 'overallfeedback_content', $assessment->id);
- if (!empty($workshop->overallfeedbackfiles)) {
- $currentdata = file_prepare_standard_filemanager($currentdata, 'feedbackauthorattachment',
- $feedbackauthorattachmentopts, $workshop->context, 'mod_workshop', 'overallfeedback_attachment',
- $assessment->id);
- }
-
- } else {
- $currentdata->feedbackauthor = file_rewrite_pluginfile_urls($currentdata->feedbackauthor, 'pluginfile.php',
- $workshop->context->id, 'mod_workshop', 'overallfeedback_content', $assessment->id);
- $currentdata->feedbackauthor = format_text($currentdata->feedbackauthor, $currentdata->feedbackauthorformat,
- array('overflowdiv' => true, 'context' => $workshop->context));
- $currentdata->feedbackauthorattachment = '-';
- if (!empty($assessment->feedbackauthorattachment)) {
- $fs = get_file_storage();
- $files = $fs->get_area_files($workshop->context->id, 'mod_workshop', 'overallfeedback_attachment', $assessment->id);
- $list = '';
- foreach ($files as $file) {
- if ($file->is_directory()) {
- continue;
- }
- $filepath = $file->get_filepath();
- $filename = $file->get_filename();
- $fileurl = moodle_url::make_pluginfile_url($workshop->context->id, 'mod_workshop', 'overallfeedback_attachment',
- $assessment->id, $filepath, $filename, true);
- $link = html_writer::link($fileurl, s($filename));
- $list .= html_writer::tag('li', $link);
- }
- $list = html_writer::tag('ul', $list, array('class' => 'overallfeedback_attachments'));
- $currentdata->feedbackauthorattachment = $list;
- }
+ if ($assessmenteditable and $workshop->overallfeedbackmode) {
+ $currentdata = file_prepare_standard_editor($currentdata, 'feedbackauthor', $workshop->overall_feedback_content_options(),
+ $workshop->context, 'mod_workshop', 'overallfeedback_content', $assessment->id);
+ if ($workshop->overallfeedbackfiles) {
+ $currentdata = file_prepare_standard_filemanager($currentdata, 'feedbackauthorattachment',
+ $workshop->overall_feedback_attachment_options(), $workshop->context, 'mod_workshop', 'overallfeedback_attachment',
+ $assessment->id);
}
-
}
$mform->set_data($currentdata);
@@ -215,14 +171,14 @@
$coredata = (object)array('id' => $assessment->id);
if (isset($data->feedbackauthor_editor)) {
$coredata->feedbackauthor_editor = $data->feedbackauthor_editor;
- $coredata = file_postupdate_standard_editor($coredata, 'feedbackauthor', $feedbackauthoreditoropts,
+ $coredata = file_postupdate_standard_editor($coredata, 'feedbackauthor', $workshop->overall_feedback_content_options(),
$workshop->context, 'mod_workshop', 'overallfeedback_content', $assessment->id);
unset($coredata->feedbackauthor_editor);
}
if (isset($data->feedbackauthorattachment_filemanager)) {
$coredata->feedbackauthorattachment_filemanager = $data->feedbackauthorattachment_filemanager;
$coredata = file_postupdate_standard_filemanager($coredata, 'feedbackauthorattachment',
- $feedbackauthorattachmentopts, $workshop->context, 'mod_workshop', 'overallfeedback_attachment',
+ $workshop->overall_feedback_attachment_options(), $workshop->context, 'mod_workshop', 'overallfeedback_attachment',
$assessment->id);
unset($coredata->feedbackauthorattachment_filemanager);
if (empty($coredata->feedbackauthorattachment)) {
View
44 mod/workshop/exassessment.php
@@ -67,6 +67,23 @@
// load the assessment form and process the submitted data eventually
$mform = $strategy->get_assessment_form($PAGE->url, 'assessment', $assessment, $assessmenteditable);
+
+// Set data managed by the workshop core, subplugins set their own data themselves.
+$currentdata = (object)array(
+ 'feedbackauthor' => $assessment->feedbackauthor,
+ 'feedbackauthorformat' => $assessment->feedbackauthorformat,
+);
+if ($assessmenteditable and $workshop->overallfeedbackmode) {
+ $currentdata = file_prepare_standard_editor($currentdata, 'feedbackauthor', $workshop->overall_feedback_content_options(),
+ $workshop->context, 'mod_workshop', 'overallfeedback_content', $assessment->id);
+ if ($workshop->overallfeedbackfiles) {
+ $currentdata = file_prepare_standard_filemanager($currentdata, 'feedbackauthorattachment',
+ $workshop->overall_feedback_attachment_options(), $workshop->context, 'mod_workshop', 'overallfeedback_attachment',
+ $assessment->id);
+ }
+}
+$mform->set_data($currentdata);
+
if ($mform->is_cancelled()) {
redirect($workshop->view_url());
} elseif ($assessmenteditable and ($data = $mform->get_data())) {
@@ -83,11 +100,34 @@
$workshop->log('update example assessment', $workshop->exassess_url($assessment->id), $assessment->submissionid);
}
}
+
+ // Let the grading strategy subplugin save its data.
$rawgrade = $strategy->save_assessment($assessment, $data);
+
+ // Store the data managed by the workshop core.
+ $coredata = (object)array('id' => $assessment->id);
+ if (isset($data->feedbackauthor_editor)) {
+ $coredata->feedbackauthor_editor = $data->feedbackauthor_editor;
+ $coredata = file_postupdate_standard_editor($coredata, 'feedbackauthor', $workshop->overall_feedback_content_options(),
+ $workshop->context, 'mod_workshop', 'overallfeedback_content', $assessment->id);
+ unset($coredata->feedbackauthor_editor);
+ }
+ if (isset($data->feedbackauthorattachment_filemanager)) {
+ $coredata->feedbackauthorattachment_filemanager = $data->feedbackauthorattachment_filemanager;
+ $coredata = file_postupdate_standard_filemanager($coredata, 'feedbackauthorattachment',
+ $workshop->overall_feedback_attachment_options(), $workshop->context, 'mod_workshop', 'overallfeedback_attachment',
+ $assessment->id);
+ unset($coredata->feedbackauthorattachment_filemanager);
+ if (empty($coredata->feedbackauthorattachment)) {
+ $coredata->feedbackauthorattachment = 0;
+ }
+ }
if ($canmanage) {
- // remember the last one who edited the reference assessment
- $DB->set_field('workshop_assessments', 'reviewerid', $USER->id, array('id' => $assessment->id));
+ // Remember the last one who edited the reference assessment.
+ $coredata->reviewerid = $USER->id;
}
+ $DB->update_record('workshop_assessments', $coredata);
+
if (!is_null($rawgrade) and isset($data->saveandclose)) {
if ($canmanage) {
redirect($workshop->view_url());
View
38 mod/workshop/form/assessment_form.php
@@ -68,29 +68,21 @@ public function definition() {
$mform->addElement('hidden', 'strategy', $this->workshop->strategy);
$mform->setType('strategy', PARAM_PLUGIN);
- if (!empty($this->workshop->overallfeedbackmode)) {
+ if ($this->workshop->overallfeedbackmode and $this->is_editable()) {
$mform->addElement('header', 'overallfeedbacksection', get_string('overallfeedback', 'mod_workshop'));
- if (!$mform->isFrozen()) {
- $mform->addElement('editor', 'feedbackauthor_editor', get_string('feedbackauthor', 'mod_workshop'), null,
- $this->options['feedbackauthoreditoropts']);
- if ($this->workshop->overallfeedbackmode == 2) {
- $mform->addRule('feedbackauthor_editor', null, 'required', null, 'client');
- }
- if (!empty($this->workshop->overallfeedbackfiles)) {
- $mform->addElement('filemanager', 'feedbackauthorattachment_filemanager',
- get_string('feedbackauthorattachment', 'mod_workshop'), null,
- $this->options['feedbackauthorattachmentopts']);
- }
-
- } else {
- $mform->addElement('static', 'feedbackauthor', get_string('feedbackauthor', 'mod_workshop'));
- if (!empty($this->workshop->overallfeedbackfiles)) {
- $mform->addElement('static', 'feedbackauthorattachment', get_string('feedbackauthorattachment', 'mod_workshop'));
- }
+ $mform->addElement('editor', 'feedbackauthor_editor', get_string('feedbackauthor', 'mod_workshop'), null,
+ $this->workshop->overall_feedback_content_options());
+ if ($this->workshop->overallfeedbackmode == 2) {
+ $mform->addRule('feedbackauthor_editor', null, 'required', null, 'client');
+ }
+ if ($this->workshop->overallfeedbackfiles) {
+ $mform->addElement('filemanager', 'feedbackauthorattachment_filemanager',
+ get_string('feedbackauthorattachment', 'mod_workshop'), null,
+ $this->workshop->overall_feedback_attachment_options());
}
}
- if (!empty($this->options['editableweight']) and !$mform->isFrozen()) {
+ if (!empty($this->options['editableweight']) and $this->is_editable()) {
$mform->addElement('header', 'assessmentsettings', get_string('assessmentweight', 'workshop'));
$mform->addElement('select', 'weight',
get_string('assessmentweight', 'workshop'), workshop::available_assessment_weights_list());
@@ -122,4 +114,12 @@ protected function definition_inner(&$mform) {
// By default, do nothing.
}
+ /**
+ * Is the form frozen (read-only)?
+ *
+ * @return boolean
+ */
+ public function is_editable() {
+ return !$this->_form->isFrozen();
+ }
}
View
145 mod/workshop/locallib.php
@@ -864,7 +864,7 @@ public function get_examples_for_reviewer($reviewerid) {
*/
public function prepare_submission(stdClass $record, $showauthor = false) {
- $submission = new workshop_submission($record, $showauthor);
+ $submission = new workshop_submission($this, $record, $showauthor);
$submission->url = $this->submission_url($record->id);
return $submission;
@@ -879,7 +879,7 @@ public function prepare_submission(stdClass $record, $showauthor = false) {
*/
public function prepare_submission_summary(stdClass $record, $showauthor = false) {
- $summary = new workshop_submission_summary($record, $showauthor);
+ $summary = new workshop_submission_summary($this, $record, $showauthor);
$summary->url = $this->submission_url($record->id);
return $summary;
@@ -893,7 +893,7 @@ public function prepare_submission_summary(stdClass $record, $showauthor = false
*/
public function prepare_example_submission(stdClass $record) {
- $example = new workshop_example_submission($record);
+ $example = new workshop_example_submission($this, $record);
return $example;
}
@@ -908,7 +908,7 @@ public function prepare_example_submission(stdClass $record) {
*/
public function prepare_example_summary(stdClass $example) {
- $summary = new workshop_example_submission_summary($example);
+ $summary = new workshop_example_submission_summary($this, $example);
if (is_null($example->grade)) {
$summary->status = 'notgraded';
@@ -945,7 +945,7 @@ public function prepare_example_summary(stdClass $example) {
*/
public function prepare_assessment(stdClass $record, $form, array $options = array()) {
- $assessment = new workshop_assessment($record, $options);
+ $assessment = new workshop_assessment($this, $record, $options);
$assessment->url = $this->assess_url($record->id);
$assessment->maxgrade = $this->real_grade(100);
@@ -983,7 +983,7 @@ public function prepare_assessment(stdClass $record, $form, array $options = arr
*/
public function prepare_example_assessment(stdClass $record, $form = null, array $options = array()) {
- $assessment = new workshop_example_assessment($record, $options);
+ $assessment = new workshop_example_assessment($this, $record, $options);
$assessment->url = $this->exassess_url($record->id);
$assessment->maxgrade = $this->real_grade(100);
@@ -1019,7 +1019,7 @@ public function prepare_example_assessment(stdClass $record, $form = null, array
*/
public function prepare_example_reference_assessment(stdClass $record, $form = null, array $options = array()) {
- $assessment = new workshop_example_reference_assessment($record, $options);
+ $assessment = new workshop_example_reference_assessment($this, $record, $options);
$assessment->maxgrade = $this->real_grade(100);
if (!empty($options['showform']) and !($form instanceof workshop_assessment_form)) {
@@ -2302,6 +2302,35 @@ public function get_gradebook_grades($userid) {
return false;
}
+ /**
+ * Return the editor options for the overall feedback for the author.
+ *
+ * @return array
+ */
+ public function overall_feedback_content_options() {
+ return array(
+ 'subdirs' => 0,
+ 'maxbytes' => $this->overallfeedbackmaxbytes,
+ 'maxfiles' => $this->overallfeedbackfiles,
+ 'changeformat' => 1,
+ 'context' => $this->context,
+ );
+ }
+
+ /**
+ * Return the filemanager options for the overall feedback for the author.
+ *
+ * @return array
+ */
+ public function overall_feedback_attachment_options() {
+ return array(
+ 'subdirs' => 1,
+ 'maxbytes' => $this->overallfeedbackmaxbytes,
+ 'maxfiles' => $this->overallfeedbackfiles,
+ 'return_types' => FILE_INTERNAL,
+ );
+ }
+
////////////////////////////////////////////////////////////////////////////////
// Internal methods (implementation details) //
////////////////////////////////////////////////////////////////////////////////
@@ -2996,14 +3025,20 @@ public function get_examples() {
/* @var array of columns from workshop_submissions that are assigned as properties */
protected $fields = array();
+ /** @var workshop */
+ protected $workshop;
+
/**
* Copies the properties of the given database record into properties of $this instance
*
+ * @param workshop $workshop
* @param stdClass $submission full record
* @param bool $showauthor show the author-related information
* @param array $options additional properties
*/
- public function __construct(stdClass $submission, $showauthor = false) {
+ public function __construct(workshop $workshop, stdClass $submission, $showauthor = false) {
+
+ $this->workshop = $workshop;
foreach ($this->fields as $field) {
if (!property_exists($submission, $field)) {
@@ -3216,15 +3251,20 @@ class workshop_example_submission extends workshop_example_submission_summary im
/* @var array of columns that are assigned as properties */
protected $fields = array();
+ /** @var workshop */
+ protected $workshop;
+
/**
* Copies the properties of the given database record into properties of $this instance
*
* The $options keys are: showreviewer, showauthor
+ * @param workshop $workshop
* @param stdClass $assessment full record
* @param array $options additional properties
*/
- public function __construct(stdClass $record, array $options = array()) {
+ public function __construct(workshop $workshop, stdClass $record, array $options = array()) {
+ $this->workshop = $workshop;
$this->validate_raw_record($record);
foreach ($this->fields as $field) {
@@ -3304,9 +3344,94 @@ class workshop_assessment extends workshop_assessment_base implements renderable
/** @var float */
public $gradinggradeover;
+ /** @var string */
+ public $feedbackauthor;
+
+ /** @var int */
+ public $feedbackauthorformat;
+
+ /** @var int */
+ public $feedbackauthorattachment;
+
/** @var array */
protected $fields = array('id', 'submissionid', 'weight', 'timecreated',
- 'timemodified', 'grade', 'gradinggrade', 'gradinggradeover');
+ 'timemodified', 'grade', 'gradinggrade', 'gradinggradeover', 'feedbackauthor',
+ 'feedbackauthorformat', 'feedbackauthorattachment');
+
+ /**
+ * Format the overall feedback text content
+ *
+ * False is returned if the overall feedback feature is disabled. Null is returned
+ * if the overall feedback content has not been found. Otherwise, string with
+ * formatted feedback text is returned.
+ *
+ * @return string|bool|null
+ */
+ public function get_overall_feedback_content() {
+
+ if ($this->workshop->overallfeedbackmode == 0) {
+ return false;
+ }
+
+ if (trim($this->feedbackauthor) === '') {
+ return null;
+ }
+
+ $content = format_text($this->feedbackauthor, $this->feedbackauthorformat,
+ array('overflowdiv' => true, 'context' => $this->workshop->context));
+ $content = file_rewrite_pluginfile_urls($content, 'pluginfile.php', $this->workshop->context->id,
+ 'mod_workshop', 'overallfeedback_content', $this->id);
+
+ return $content;
+ }
+
+ /**
+ * Prepares the list of overall feedback attachments
+ *
+ * Returns false if overall feedback attachments are not allowed. Otherwise returns
+ * list of attachments (may be empty).
+ *
+ * @return bool|array of stdClass
+ */
+ public function get_overall_feedback_attachments() {
+
+ if ($this->workshop->overallfeedbackmode == 0) {
+ return false;
+ }
+
+ if ($this->workshop->overallfeedbackfiles == 0) {
+ return false;
+ }
+
+ if (empty($this->feedbackauthorattachment)) {
+ return array();
+ }
+
+ $attachments = array();
+ $fs = get_file_storage();
+ $files = $fs->get_area_files($this->workshop->context->id, 'mod_workshop', 'overallfeedback_attachment', $this->id);
+ foreach ($files as $file) {
+ if ($file->is_directory()) {
+ continue;
+ }
+ $filepath = $file->get_filepath();
+ $filename = $file->get_filename();
+ $fileurl = moodle_url::make_pluginfile_url($this->workshop->context->id, 'mod_workshop',
+ 'overallfeedback_attachment', $this->id, $filepath, $filename, true);
+ $previewurl = new moodle_url(moodle_url::make_pluginfile_url($this->workshop->context->id, 'mod_workshop',
+ 'overallfeedback_attachment', $this->id, $filepath, $filename, false), array('preview' => 'bigthumb'));
+ $attachments[] = (object)array(
+ 'filepath' => $filepath,
+ 'filename' => $filename,
+ 'fileurl' => $fileurl,
+ 'previewurl' => $previewurl,
+ 'mimetype' => $file->get_mimetype(),
+
+ );
+ }
+
+ return $attachments;
+ }
}
View
65 mod/workshop/renderer.php
@@ -673,6 +673,10 @@ protected function render_workshop_assessment(workshop_assessment $assessment) {
get_string('assessmentform', 'workshop'), '', false, true);
$o .= $this->output->container(self::moodleform($assessment->form), 'assessment-form');
$o .= print_collapsible_region_end(true);
+
+ if (!$assessment->form->is_editable()) {
+ $o .= $this->overall_feedback($assessment);
+ }
}
$o .= $this->output->container_end(); // main wrapper
@@ -701,6 +705,67 @@ protected function render_workshop_example_reference_assessment(workshop_example
}
/**
+ * Renders the overall feedback for the author of the submission
+ *
+ * @param workshop_assessment $assessment
+ * @return string HTML
+ */
+ protected function overall_feedback(workshop_assessment $assessment) {
+
+ $content = $assessment->get_overall_feedback_content();
+
+ if ($content === false) {
+ return '';
+ }
+
+ $o = '';
+
+ if (!is_null($content)) {
+ $o .= $this->output->container($content, 'content');
+ }
+
+ $attachments = $assessment->get_overall_feedback_attachments();
+
+ if (!empty($attachments)) {
+ $o .= $this->output->container_start('attachments');
+ $images = '';
+ $files = '';
+ foreach ($attachments as $attachment) {
+ $icon = $this->output->pix_icon(file_file_icon($attachment), get_mimetype_description($attachment),
+ 'moodle', array('class' => 'icon'));
+ $link = html_writer::link($attachment->fileurl, $icon.' '.substr($attachment->filepath.$attachment->filename, 1));
+ if (file_mimetype_in_typegroup($attachment->mimetype, 'web_image')) {
+ $preview = html_writer::empty_tag('img', array('src' => $attachment->previewurl, 'alt' => '', 'class' => 'preview'));
+ $preview = html_writer::tag('a', $preview, array('href' => $attachment->fileurl));
+ $images .= $this->output->container($preview);
+ } else {
+ $files .= html_writer::tag('li', $link, array('class' => $attachment->mimetype));
+ }
+ }
+ if ($images) {
+ $images = $this->output->container($images, 'images');
+ }
+
+ if ($files) {
+ $files = html_writer::tag('ul', $files, array('class' => 'files'));
+ }
+
+ $o .= $images.$files;
+ $o .= $this->output->container_end();
+ }
+
+ if ($o === '') {
+ return '';
+ }
+
+ $o = $this->output->box($o, 'overallfeedback');
+ $o = print_collapsible_region($o, 'overall-feedback-wrapper', uniqid('workshop-overall-feedback'),
+ get_string('overallfeedback', 'workshop'), '', false, true);
+
+ return $o;
+ }
+
+ /**
* Renders a perpage selector for workshop listings
*
* The scripts using this have to define the $PAGE->url prior to calling this
View
22 mod/workshop/styles.css
@@ -383,7 +383,8 @@
display: inline;
}
-.path-mod-workshop .assessment-full .assessment-form-wrapper {
+.path-mod-workshop .assessment-full .assessment-form-wrapper,
+.path-mod-workshop .assessment-full .overall-feedback-wrapper {
margin-top: 0.5em;
padding: 0px 1em;
}
@@ -399,6 +400,25 @@
}
/**
+ * Overall feedback
+ */
+.path-mod-workshop .assessment-full .overallfeedback .content,
+.path-mod-workshop .assessment-full .overallfeedback .attachments {
+ padding: 5px 10px;
+}
+
+.path-mod-workshop .assessment-full .overallfeedback .attachments .files img.icon {
+ margin-right: 5px;
+}
+
+.path-mod-workshop .assessment-full .overallfeedback .attachments .images div {
+ display: inline-block;
+ margin: 5px;
+ padding: 5px;
+ border: 1px solid #ddd;
+}
+
+/**
* Assessment form
*/
.path-mod-workshop .assessmentform .description {

0 comments on commit 77d746a

Please sign in to comment.
Something went wrong with that request. Please try again.