I have finished making changes based on all your comments below. I don't see how to send this back to peer review as you described though (it looks like it is still in peer review - the only option I have is "Finish Peer Review").
Here is a summary of the changes:
Detailed list of notes from first set of comments:
- Name of old assignment module - asked MD waiting for a response
- index.php now does a listing of assignments in the course
- New context API is used
- style.css renamed to styles.css
- Now using MUST_EXIST and removed error checking
- Moved view capability check to the entry page
- lib.php no longer includes locallib.php
- all require_onces calls use absolute paths
- no underscores in variable names
- no short variable names
- pass by reference removed
- unused variables and globals removed
- format_string and format_text always pass context as option
- camel casing removed form language strings
- All classes passed to a function are correctly documented in the phpdocs and the type hinting.
Detailed list of notes from second set of comments:
1. Cannot move this entirely to mod_form because it must allow the plugins to add their specific settings to the setting page (add_all_plugin_settings). I did however move all the stuff from add_settings to mod_form and then just call add_all_plugin_settings on the assignment.
2. assignment::get_course_context uses context::get_course_context
3. get_courseid_from_context has been removed
4. list_enrolled_users_with_capability has been renamed to list_participants and I added useridsonly as an parameter
5. now called count_participants and it uses count_enrolled_users
6. The constructor for the grading table only gets called once on the grading page now.
7. ASSIGN_PLUGIN_CLASS_FILE is gone
8. No longer uses basename and just gets the name from the key
9. I don't think that sorting thing will be a problem. When the plugins call move - it sets the order for all plugins sequentially (so they are self correcting). I think the worst case will be that there are holes in the order for plugins that get removed.
10. This has been changed to only load assignment->instance from the database. The form is explicitly passed as a variable for the add and updates methods now.
11. Error handling for delete_instance cleaned up, rebuild_course_cache has been removed and events are not deleted.
12. Delete still needs to work out the context as it is used to do things like cleanup files and is made available to the plugins to delete their data (which has also been added).
13. Changed to use a static variable cache for these strings
14. Now throws coding_exception
15. Now throws coding_exception
16. can_grade is now only checking one capability
17. redirect now gets a moodle_url
18. Agreed - I did the same for get_grade
19. The view functions now all return their output as a string which is written from the view page. I take your point about this allowing the calling code to chose to modify the output or choose when to write it, but this approach will generally require more memory and there is all ready an oportunity to modify the output through the renderers. This also requires ob_start/end hacks to capture the output from things like moodleform, the plagiarism api and flexible_table.
20. I can't see how this can be simplified any further - any suggestions?
21. Yes it can - that function came before $this->output and hadn't been updated.