Add the ability to process Mahara HTML Lite portfolios #153
Conversation
Dependencies: * Mahara with HTML Lite export webservice configured * assignsubmission_maharaws plugin with commit f70d1c0 (Show Ouriginal plagirism check links)
| require_once($CFG->dirroot . '/mod/assign/submissionplugin.php'); | ||
| require_once($CFG->dirroot . '/mod/assign/submission/maharaws/locallib.php'); | ||
| $client = new \mahara_oauth($args); | ||
| if (!empty($CFG->disablesslchecks)) { |
There was a problem hiding this comment.
what's this for? - typically I have a hate for this sort of workaround... especailly if it's for some internal testing site... just use proper SSL certs - don't let people avoid them.
| global $DB; | ||
| $plagiarismfile = urkund_get_plagiarism_file($cmid, $userid, $file, $relateduserid); | ||
| $plagiarismvalues = $DB->get_records_menu('plagiarism_urkund_config', array('cm' => $cmid), '', 'name, value'); | ||
|
|
||
| // Check if $plagiarismfile needs to be downloaded from Mahara. |
There was a problem hiding this comment.
this comment doesn't really explain what's going on here... "if statusoveride is passed, update the record with the new value - used by mahara" .. or something along those lines...
| * Function called by scheduled task to get files from Mahara. | ||
| * | ||
| */ | ||
| function plagiarism_urkund_fetch_mahara() { |
There was a problem hiding this comment.
I'd be more inclined to drop this code into the task itself rather than a function within lib.php - nothing else will be calling this except the task right?
| if (!empty($submissionplugins) && key_exists('maharaws', $submissionplugins)) { | ||
| $mform->addElement('html', get_string('setmahara', 'plagiarism_urkund')); | ||
|
|
||
| $mform->addElement('text', 'urkund_maharawsurl', get_string('maharawsurl', 'plagiarism_urkund'), |
There was a problem hiding this comment.
storing this stuff at the activity level and allowing the teacher to configure feels very wrong - we should not be exposing secrets/keys to normal teaching staff.
The keys/urls secrets should be just stored at the admin level - not within the course module editing page. I suspect DCU only have a single mahara instance in use so storing this in one place shouldn't be a problem for them.
If we need to support multiple Mahara instances from the same Moodle site, these should still be set up at the site level, and then a drop list shown on the course module editing page allowing the teacher to select which Mahara site they are using, pulling the keys/urls from site-level settings.
There was a problem hiding this comment.
For initial release, I'd reccomend removing these from the teachers settings completly and just give them the abilty to turn "get mahara stuff" on/off as a single setting.
| 'userid' => $this->userid, | ||
| 'statuscode' => 'maharawsdl' | ||
| )); | ||
| // Remove saved zip file from storage. |
There was a problem hiding this comment.
This feels like it could leave a bunch of un-used zip files I'm not sure we need to wrap this in a call to if ($success) unless debugging - for example - if urkund_queue_file returns '' because the file isn't one we care about etc.
| @@ -632,6 +637,24 @@ public function event_handler($eventdata) { | |||
| $result = true; | |||
| if (isset($plagiarismvalues['urkund_draft_submit']) && | |||
| $plagiarismvalues['urkund_draft_submit'] == PLAGIARISM_URKUND_DRAFTSUBMIT_FINAL) { | |||
|
|
|||
| // If there's a Mahara portfolio submission, add it to the queue. | |||
| $submissionid = $eventdata['objectid']; | |||
There was a problem hiding this comment.
This feels a bit fragile - can we check that mahara submission is enabled in this assignment first?
|
|
||
| // Check if there's a zip file already downloaded by previous processing. | ||
| $fs = get_file_storage(); | ||
| $fileinfo = array( |
There was a problem hiding this comment.
I feel like we be setting userid in this array - but I haven't checked to see if it's setting the correct userid later on in the code to the student (owner of the file.)
also - as this is new user data we are storing within the moodle site we'll need to implement privacy api functions to allow retrieiving and deleting of these files. (hopefully just the individual content files, as we're deleting the zip files after using them right?)
| $string['setmahara'] = 'Set the Mahara URL, and the HTML Lite OAuth secret and key here to check portfolio submissions.'; | ||
| $string['maharawshtmlliteenabled'] = 'Check Mahara portfolios'; | ||
| $string['maharawshtmllitekey'] = 'Mahara HTML Lite OAuth key'; | ||
| $string['maharawshtmllitekey_help'] = 'The HTML Lite OAuth key from the partner Mahara sute.'; |
There was a problem hiding this comment.
"sute" - > "site" ? :-) - few other examples below... I'd also be tempted to remove the word "Partner" from those and just make it "the Mahara site"
No description provided.