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

Hi Dan and Kanika,
The patch looks great. Here are a few small things to consider:-

1. get_context_instance is deprecated. context_module::instance should be used instead.
2. PHP doc for the class forum_trigger_content_uploaded_event is missing @return.
3. I guess it will be better to use empty instead of if ($CFG->enableplagiarism) unless we are sure it is always set
4. How does this patch deals with file alias?

Just a note for discussion:-
Shouldn't we have different triggers for upload and edit?

Thanks

Hi Ankit,

Thanks for your suggestions, I have modified the code accordingly.
The event "assessable_content_uploaded" contains both message as well as file.

We are triggering the event whenever a post is added/updated. On triggering the event "assessable_content_uploaded", plagiarism plugin will check if that data has already been submitted, if yes then it will not send the file for detection. What is your idea behind different triggers?

Thanks,
Kanika

thanks Ankit/Kanika,

Ankit is suggesting that there should probably be a way to distinguish between an upload and an edit event - we may not need this for plagiarism plugins but it makes sense to future proof for other plugins that might use these events.
maybe we could do this by adding an extra param to forum_trigger_content_uploaded_event that sets $eventdata->name for each type of event triggered - we could use the function names they come from eg "forum_add_discussion", "forum_add_new_post" etc.

make sense?

Thanks Dan for explaining that. Yeah it makes sense to use function names. Also if we decide to do this here, it needs to be done with assign and assignments as well IMHO.
By file aliases I meant, in 2.3+ you can use shortcut to a actual file, without actually uploading it. Say I have a file in private files area, I can simply create a shortcut and use that as forum attachment in a forum post. Am not sure how the current patch is treating(or is supposed to treat) such cases.

Hi Ankit, afaik the use of aliases is fine - get_area_files returns valid pathnamehashes which are used to access the files by the plagiarism plugins themselves.

Kanika - can you please add an $eventdata->name to the other "new" events being added - once these have been integrated we will add a patch to add ->name to existing assessable events to keep them consistent.

Hi Dan,

I have added $eventdata->name by setting it as function name from where it is called in forum posts, but what should I set in other modules like in online assignment where event is called from a function named "update_submission".

Thanks,
Kanika

What are your thoughts on this patch ankit? - Kanika will email you about the other functions.

Looks good to me.
+1 for integration.
Thanks

Thanks guys, this has been integrated now

Thanks for fixing this Kanika,

Was able to create forum, post to it with attachments and view it as another user.

*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!