Add Plagiarism api support to new assignment with online text

Hi Dan,

Here is the possible solution - https://github.com/kanikagoyal/moodle/compare/master_MDL-34131_add_plagiarism_api_support_for_new_online_assignment

Please have a look and let me know what all changes are required.

Thanks,
Kanika

passing this up for peer review - would be good to get someone elses eyes on this too.

Hi Dan and Kanika,
A few questions:-

1. plagiarism_print_disclosure is not needed?
2. Shouldn't we process the file attachments as well?

The patch looks great.
Thanks

Thanks Ankit - print_disclosure should be there already as part of the normal assignment types.

Yes - attachements should be there, I think there's been a miscommunication between me and Kanika on the requirements here - Kanika I'll send you an e-mail about it.

Hi Dan,

Updated the link. Sorry for the delay.
Please review it again.

Thanks,
Kanika

great - looks good to me, bouncing up for integration - thanks.

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Hi,

Damyon is the component maintainer of mod_assign and hasn't been involved in this issue, so pinging him for a +1.

thanks

Thanks Dan, I had missed this because of the status.

Comments below:

   $files = $fs->get_area_files($this->assignment->get_context()->id, 'assignsubmission_file', ASSIGNSUBMISSION_FILE_FILEAREA, $submission->id, "id", false);

This is wrong - the online text plugin should only be dealing with it's own files (linked images etc).

Should be:

   $files = $fs->get_area_files($this->assignment->get_context()->id, 'assignsubmission_onlinetext', ASSIGNSUBMISSION_ONLINETEXT_FILEAREA, $submission->id, "id", false);

Also - the 'content' parameter passed to get_links should be trimmed and formatted (at least this is what the old onlinetext assignment does).

From the old assignment:

'content' => trim(format_text($submission->data1, $submission->data2, array('context' => $context))), 

Other than those issues I am happy with this patch.

Regards, Damyon

Actually one more thing (possibly should be a different issue). Can the list of valid parameters for get_links be documented somewhere? The array is passed directly to the plagiarism plugin(s) and as there are no plugins in core - and the parameters are not listed on the dev page, it is only possible to guess the parameters by looking at the code in the old assignment module.

Thanks Damyon, I'm reopening this based on those comments which need to be addressed.

Thanks Dan/Damyon - whoops, will try to add Damyon to any future mod/assign stuff - Kanika can you please make the changes Damyon suggests? - thanks!

Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

Thanks Damyon for pointing that out. I have updated the link.

Thanks,
Kanika

Thanks Rosie - Damyon can you please take a look at the updated patch and confirm you're ok for this to go up for integration?

thanks!

Those changes look good. +1 from me - thanks!

thanks Damyon

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Thanks guys, i've integrated this now.

Hi guys, while testing I had this notice on the page following the student submission.

Notice: Trying to get property of non-object in /home/fred/www/repositories/testing_master/moodle/mod/assign/submission/onlinetext/locallib.php on line 140

Call Stack:
    0.0003     661944   1. {main}() /home/fred/www/repositories/testing_master/moodle/mod/assign/view.php:0
    0.1833   54305056   2. assign->view() /home/fred/www/repositories/testing_master/moodle/mod/assign/view.php:53
    0.1833   54305416   3. assign->process_save_submission() /home/fred/www/repositories/testing_master/moodle/mod/assign/locallib.php:303
    0.2467   56076288   4. assign_submission_onlinetext->save() /home/fred/www/repositories/testing_master/moodle/mod/assign/locallib.php:2827


Notice: Trying to get property of non-object in /home/fred/www/repositories/testing_master/moodle/mod/assign/submission/onlinetext/locallib.php on line 140

Call Stack:
    0.0003     661944   1. {main}() /home/fred/www/repositories/testing_master/moodle/mod/assign/view.php:0
    0.1833   54305056   2. assign->view() /home/fred/www/repositories/testing_master/moodle/mod/assign/view.php:53
    0.1833   54305416   3. assign->process_save_submission() /home/fred/www/repositories/testing_master/moodle/mod/assign/locallib.php:303
    0.2467   56076288   4. assign_submission_onlinetext->save() /home/fred/www/repositories/testing_master/moodle/mod/assign/locallib.php:2827

Apart from that, the test is all good. Should I pass, fail or wait for a quick fix?

I'd consider that a fail but I should be able to push a quick fix in about 3 hours when I get back to my desktop machine unless someone else beats me to it

just pushed a fix - can an integrator please pull in the updated code? - it was using the wrong vars when triggering the event. thanks!

done

Test successful on master. Cheers!

Fixed STOP Closed STOP Thanks STOP

Yay, imagination! Ciao