Add Plagiarism api support to Online assignment type

would be good for someone else to peer review Kanika's work here too..

Ideally I'd like to get this into 2.2Stable and master if poss although I expect mod/assignment will be removed from 2.4 or 2.5 sometime soon.

thanks!

Hi Kanika and Dan,
The patch looks great. Here are a few suggestions to fix/discuss:-

1. $course_context should be $coursecontext. I don't think we allow _ in variable names.
2. 'userid'=>$USER->id should be 'userid' => $USER->id (Mark the spaces) - This needs to be updated for all similar instances.
3. should we be not doing the event trigger for other assignment types?
4. will it be better to use html writer class for outputing the plagiarism_get_links() ?
5. shouldn't we trigger the hook when the content is actually saved (after $this->update_submission()) instead of doing it later after the redirect to the same page? Say user edited his submission and is redirected to view.php?Saved=1 . Now the user reloads the page, it will trigger the event hook again although, there is no submission made.

Thanks

Thanks for the feedback Ankit - the other assignment types already have this event trigger for the plagiarism api, it's the online assignment type that doesn't have it yet.
using html_writer to output plagiarism_get_links sounds like an ok idea but we should probably tackle that separately and if we decide to use it, update all existing calls to plagiarism_get_links in core code.

I hadn't looked closely at where the event was added yet - good spotting! - Kanika can you please move the event trigger just after update_submission is called? - thanks!

Hi Dan and Ankit,
Thanks for your feedback and suggestions. I have changed the code as per the suggestions and updated the link too.
Please have a look again and let me know if further changes are required.

Thanks,
Kanika

Hi Kanika,
Looks good to me. Just as Dan said, make sure these lines are not too lengthy. You might want to install and use the code checker plugins. Helps a lot with coding guidelines. https://github.com/moodlehq/moodle-local_codechecker

Thanks

great! - pushing this one up for integration.
Note to integrator: MASTER only please.

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, this has been integrated now

I don' think that this has been integrated yet as the commit message doesn't appear in git log.

looks like it's there to me - I see Sams merge branch commit and Kanika's patch later - this page lists it:
http://git.moodle.org/gw?p=integration.git;a=shortlog

just search for '32228' and you should see both

I've just checked master as well.

git log --oneline --grep='MDL-32228' ./
cbbdc7d Merge branch 'master_MDL-32228_add_plagiarism_api_support_to_online_assignment_type' of git://github.com/kanikagoyal/moodle
93d52d8 MDL-32228: Plagiarism API - add support for plagiarism api to online assignment type

Looks like its definitely there Tim.

I've just tried git log | grep MDL-32228 again this morning and it is indeed there. I did get Dan P to look for it yesterday also and he couldn't find it either. Sam will you reset this so I can test it?

Reset to testing

Sorry, we must've been grepping the wrong thing! :S

Nope, I just checked my command history via cli using the up arrow and we were definitely greping the right thing.

*Notice*: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26

Congratulations

!

You've made into Moodle

+

I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world.

cheers!