Details
Minor
Description
load_users() in grade/report/grader/lib.php uses role assignments but doesn't use the enrollment tables to determine who should be displayed on the grader report. See calculate_averages() in grade/report/user/lib.php committed as part of MDL-20617 for an example.
Grep the whole codebase looking for other instances of "gradebookroles" and update them all to use the enrollment tables in addition to the gradeable roles.
Gliffy Diagrams
Attachments
Activity
- All
- Comments
- Work Log
- History
- Activity
- Links Hierarchy
After much more further discussion I'm reopening and fixing this as a separate item.
This isnt close to complete but you can see it progress by going to https://github.com/andyjdavis/moodle/compare/master...MDL-25769_enrollment
Think Ive fixed them all.
repo: git://github.com/andyjdavis/moodle.git
branch: MDL-25769_enrollment
diff: https://github.com/andyjdavis/moodle/compare/master...MDL-25769_enrollment
Raised MDL-25879 while doing this. Looks like theres some redundant code.
Hi Andrew,
Just looking through this now, the following are the things I've spotted so far:
- grade/import/lib.php
- ln 170: You use $this out of scope.
- ln 178: White space (not added by you)
- ln 186: $params is forgotten about, but looks like its still required "WHERE giv.importcode = ?"
- grade/report/grader/lib.php
- ln 1314: $params already contains courseid; I don't think this line is needed any more.
Grader report tested fine, I'm guessing the import will fail because of the above so I didn't test that.
Cheers
Sam
Thanks for you keen eyes Sam. I've fixed that stuff although Im unable to test the changes in grade import. I've raised MDL-25887 as something is broken in grade export/import. Note that I was able to reproduce the import errors in both this branch and another branch so it isn't a problem introduced by these changes.
Hi Andrew,
Thanks for making the requested changes, a couple of things though:
- You still have $this->context being used in grade/import/lib.php line 171
- Given that the database query is using named params its probably best to change giv.importcode = ? to giv.importcode = :importcode and then refactor $params as such.
Rest looks fine, pitty we can't test the grade import code however.
I wonder whether that area of the patch should be split out either against the issue you've created to fix the import, or to a separate issue? You may want to check with Petr and see what he thinks.
Cheers
Sam
Thats weird. My reply to this seems to have disappeared.
Anyhow, Ive fixed those 2 issues. Ive also raised submitted a fix for MDL-25887. I'm not keen to spend anymore time shuffling changes between branches than absolutely necessary.
Submitting for consideration by the integrators. PULL-102
committed, thanks
Fixing this as part of
MDL-25769after all