Skip to content

Commit

Permalink
MDL-60008 assignfeedback_editpdf: Show warnings
Browse files Browse the repository at this point in the history 
When not all files are available in the online pdf, display a warning to graders
that some files must be downloaded.
  • Loading branch information
Damyon Wiese committed Dec 6, 2018
1 parent 0888ebc commit 86c5770
Show file tree
Hide file tree
Showing 12 changed files with 150 additions and 27 deletions.
9 changes: 7 additions & 2 deletions mod/assign/feedback/editpdf/ajax.php
View file
Expand Up @@ -70,17 +70,21 @@
'filecount' => 0,
'pagecount' => 0,
'pageready' => 0,
'partial' => false,
'pages' => [],
];

$combineddocument = document_services::get_combined_document_for_attempt($assignment, $userid, $attemptnumber);
$response->status = $combineddocument->get_status();
$response->filecount = $combineddocument->get_document_count();

if ($response->status === combined_document::STATUS_READY) {
$readystatuslist = [combined_document::STATUS_READY, combined_document::STATUS_READY_PARTIAL];
$completestatuslist = [combined_document::STATUS_COMPLETE, combined_document::STATUS_FAILED];

if (in_array($response->status, $readystatuslist)) {
$combineddocument = document_services::get_combined_pdf_for_attempt($assignment, $userid, $attemptnumber);
$response->pagecount = $combineddocument->get_page_count();
} else if ($response->status === combined_document::STATUS_COMPLETE || $response->status === combined_document::STATUS_FAILED) {
} else if (in_array($response->status, $completestatuslist)) {
$pages = document_services::get_page_images_for_attempt($assignment,
$userid,
$attemptnumber,
Expand All @@ -95,6 +99,7 @@
if ($readonly) {
$filearea = document_services::PAGE_IMAGE_READONLY_FILEAREA;
}
$response->partial = $combineddocument->is_partial_conversion();

foreach ($pages as $id => $pagefile) {
$index = count($response->pages);
Expand Down
53 changes: 45 additions & 8 deletions mod/assign/feedback/editpdf/classes/combined_document.php
View file
Expand Up @@ -44,6 +44,11 @@ class combined_document {
*/
const STATUS_READY = 1;

/**
* Status value representing all documents are ready to be combined as are supported.
*/
const STATUS_READY_PARTIAL = 3;

/**
* Status value representing a successful conversion.
*/
Expand Down Expand Up @@ -100,6 +105,7 @@ public function get_status() {
}

$pending = false;
$partial = false;
foreach ($this->sourcefiles as $file) {
// The combined file has not yet been generated.
// Check the status of each source file.
Expand All @@ -110,12 +116,20 @@ public function get_status() {
case \core_files\conversion::STATUS_PENDING:
$pending = true;
break;

// There are 4 status flags, so the only remaining one is complete which is fine.
case \core_files\conversion::STATUS_FAILED:
$partial = true;
break;
}
}
}
if ($pending) {
return self::STATUS_PENDING_INPUT;
} else {
if ($partial) {
return self::STATUS_READY_PARTIAL;
}
return self::STATUS_READY;
}
}
Expand All @@ -131,6 +145,20 @@ public function set_combined_file($file) {
return $this;
}

/**
* Return true of the combined file contained only some of the submission files.
*
* @return boolean
*/
public function is_partial_conversion() {
$combinedfile = $this->get_combined_file();
if (empty($combinedfile)) {
return false;
}
$filearea = $combinedfile->get_filearea();
return $filearea == document_services::PARTIAL_PDF_FILEAREA;
}

/**
* Retrieve the completed combined file.
*
Expand Down Expand Up @@ -210,11 +238,12 @@ public function combine_files($contextid, $itemid) {
global $CFG;

$currentstatus = $this->get_status();
$readystatuslist = [self::STATUS_READY, self::STATUS_READY_PARTIAL];
if ($currentstatus === self::STATUS_FAILED) {
$this->store_empty_document($contextid, $itemid);

return $this;
} else if ($currentstatus !== self::STATUS_READY) {
} else if (!in_array($currentstatus, $readystatuslist)) {
// The document is either:
// * already combined; or
// * pending input being fully converted; or
Expand Down Expand Up @@ -273,7 +302,7 @@ public function combine_files($contextid, $itemid) {
}

// Store the newly created file as a stored_file.
$this->store_combined_file($tmpfile, $contextid, $itemid);
$this->store_combined_file($tmpfile, $contextid, $itemid, ($currentstatus == self::STATUS_READY_PARTIAL));

// Note the verified page count.
$this->pagecount = $verifypagecount;
Expand All @@ -298,11 +327,12 @@ protected function mark_combination_failed() {
* @param string $tmpfile The path to the file on disk to be stored.
* @param int $contextid The contextid for the file to be stored under
* @param int $itemid The itemid for the file to be stored under
* @param boolean $partial The combined pdf contains only some of the source files.
* @return $this
*/
protected function store_combined_file($tmpfile, $contextid, $itemid) {
protected function store_combined_file($tmpfile, $contextid, $itemid, $partial = false) {
// Store the file.
$record = $this->get_stored_file_record($contextid, $itemid);
$record = $this->get_stored_file_record($contextid, $itemid, $partial);
$fs = get_file_storage();

// Delete existing files first.
Expand Down Expand Up @@ -352,12 +382,14 @@ public function get_page_count() {
return $this->pagecount;
}

if ($this->get_status() === self::STATUS_FAILED) {
$status = $this->get_status();

if ($status === self::STATUS_FAILED) {
// The empty document will be returned.
return 1;
}

if ($this->get_status() !== self::STATUS_COMPLETE) {
if ($status !== self::STATUS_COMPLETE) {
// No pages yet.
return 0;
}
Expand Down Expand Up @@ -394,13 +426,18 @@ public function get_document_count() {
*
* @param int $contextid The contextid for the file to be stored under
* @param int $itemid The itemid for the file to be stored under
* @param boolean $partial The combined file contains only some of the source files.
* @return stdClass
*/
protected function get_stored_file_record($contextid, $itemid) {
protected function get_stored_file_record($contextid, $itemid, $partial = false) {
$filearea = document_services::COMBINED_PDF_FILEAREA;
if ($partial) {
$filearea = document_services::PARTIAL_PDF_FILEAREA;
}
return (object) [
'contextid' => $contextid,
'component' => 'assignfeedback_editpdf',
'filearea' => document_services::COMBINED_PDF_FILEAREA,
'filearea' => $filearea,
'itemid' => $itemid,
'filepath' => '/',
'filename' => document_services::COMBINED_PDF_FILENAME,
Expand Down
13 changes: 12 additions & 1 deletion mod/assign/feedback/editpdf/classes/document_services.php
View file
Expand Up @@ -42,6 +42,8 @@ class document_services {
const FINAL_PDF_FILEAREA = 'download';
/** File area for combined pdf */
const COMBINED_PDF_FILEAREA = 'combined';
/** File area for partial combined pdf */
const PARTIAL_PDF_FILEAREA = 'partial';
/** File area for importing html */
const IMPORT_HTML_FILEAREA = 'importhtml';
/** File area for page images */
Expand Down Expand Up @@ -261,15 +263,23 @@ public static function get_combined_document_for_attempt($assignment, $userid, $
$submission = $assignment->get_user_submission($userid, false, $attemptnumber);
}


$contextid = $assignment->get_context()->id;
$component = 'assignfeedback_editpdf';
$filearea = self::COMBINED_PDF_FILEAREA;
$partialfilearea = self::PARTIAL_PDF_FILEAREA;
$itemid = $grade->id;
$filepath = '/';
$filename = self::COMBINED_PDF_FILENAME;
$fs = get_file_storage();

$combinedpdf = $fs->get_file($contextid, $component, $filearea, $itemid, $filepath, $filename);
$partialpdf = $fs->get_file($contextid, $component, $partialfilearea, $itemid, $filepath, $filename);
if (!empty($partialpdf)) {
$combinedpdf = $partialpdf;
} else {
$combinedpdf = $fs->get_file($contextid, $component, $filearea, $itemid, $filepath, $filename);
}

if ($combinedpdf && $submission) {
if ($combinedpdf->get_timemodified() < $submission->timemodified) {
// The submission has been updated since the PDF was generated.
Expand Down Expand Up @@ -381,6 +391,7 @@ protected static function generate_page_images_for_attempt($assignment, $userid,

$tmpdir = \make_temp_directory('assignfeedback_editpdf/pageimages/' . self::hash($assignment, $userid, $attemptnumber));
$combined = $tmpdir . '/' . self::COMBINED_PDF_FILENAME;

$document->get_combined_file()->copy_content_to($combined); // Copy the file.

$pdf = new pdf();
Expand Down
3 changes: 2 additions & 1 deletion mod/assign/feedback/editpdf/classes/renderer.php
View file
Expand Up @@ -268,7 +268,8 @@ public function render_assignfeedback_editpdf_widget(assignfeedback_editpdf_widg
'stamp',
'stamppicker',
'cannotopenpdf',
'pagenumber'
'pagenumber',
'partialwarning'
), 'assignfeedback_editpdf');

return $html;
Expand Down
3 changes: 1 addition & 2 deletions mod/assign/feedback/editpdf/classes/task/convert_submissions.php
View file
Expand Up @@ -102,10 +102,9 @@ public function execute() {
foreach ($users as $userid) {
try {
$combineddocument = document_services::get_combined_pdf_for_attempt($assignment, $userid, $attemptnumber);
$status = $combineddocument->get_status();

switch ($combineddocument->get_status()) {
case combined_document::STATUS_READY:
case combined_document::STATUS_READY_PARTIAL:
case combined_document::STATUS_PENDING_INPUT:
// The document has not been converted yet or is somehow still ready.
continue 2;
Expand Down
1 change: 1 addition & 0 deletions mod/assign/feedback/editpdf/lang/en/assignfeedback_editpdf.php
View file
Expand Up @@ -72,6 +72,7 @@
$string['pagenumber'] = 'Page {$a}';
$string['pagexofy'] = 'Page {$a->page} of {$a->total}';
$string['pen'] = 'Pen';
$string['partialwarning'] = 'Some of the submission files must be downloaded.';
$string['pluginname'] = 'Annotate PDF';
$string['privacy:metadata:colourpurpose'] = 'Colour of the comment or annotation';
$string['privacy:metadata:conversionpurpose'] = 'Files are converted to PDFs to allow for annotations.';
Expand Down
2 changes: 1 addition & 1 deletion mod/assign/feedback/editpdf/styles.css
View file
Expand Up @@ -67,7 +67,7 @@
display: none;
position: absolute;
left: 20px;
top: 60px;
top: 120px;
}

.yui3-colourpicker-hidden,
Expand Down
29 changes: 26 additions & 3 deletions .../build/moodle-assignfeedback_editpdf-editor/moodle-assignfeedback_editpdf-editor-debug.js
View file
Expand Up @@ -3829,6 +3829,21 @@ EDITOR.prototype = {
});
},

/**
* Display an error in a small part of the page (don't block everything).
*
* @param string The error text.
* @protected
* @method warning
*/
warning: function(message) {
var drawingregion = this.get_dialogue_element(SELECTOR.DRAWINGREGION);

var warningelement = Y.Node.create('<div class="alert alert-warning" role="alert"></div>');
warningelement.setHTML(message);
drawingregion.prepend(warningelement);
},

/**
* The info about all pages in the pdf has been returned.
*
Expand All @@ -3837,7 +3852,8 @@ EDITOR.prototype = {
* @method prepare_pages_for_display
*/
prepare_pages_for_display: function(data) {
var i, j, comment, error;
var i, j, comment, error, annotation, readonly;

if (!data.pagecount) {
if (this.dialogue) {
this.dialogue.hide();
Expand All @@ -3863,11 +3879,17 @@ EDITOR.prototype = {
comment.rawtext);
}
for (j = 0; j < this.pages[i].annotations.length; j++) {
data = this.pages[i].annotations[j];
this.pages[i].annotations[j] = this.create_annotation(data.type, data);
annotation = this.pages[i].annotations[j];
this.pages[i].annotations[j] = this.create_annotation(annotation.type, annotation);
}
}

readonly = this.get('readonly');
if (!readonly && data.partial) {
// Warn about non converted files, but only for teachers.
this.warning(M.util.get_string('partialwarning', 'assignfeedback_editpdf'));
}

// Update the ui.
if (this.quicklist) {
this.quicklist.load();
Expand Down Expand Up @@ -4817,6 +4839,7 @@ M.assignfeedback_editpdf.editor.init = M.assignfeedback_editpdf.editor.init || f
"querystring-stringify-simple",
"moodle-core-notification-dialog",
"moodle-core-notification-alert",
"moodle-core-notification-warning",
"moodle-core-notification-exception",
"moodle-core-notification-ajaxexception"
]
Expand Down

0 comments on commit 86c5770

Please sign in to comment.