Sorry Jamie, but I don't like this patch.
1. It is doing two database queries for each call to quiz_report_has_view_capability, and on some pages, there will be several such calls. That is really bad for performance. (Sure, we could add caching, but ...)
2. We have to refer to some of the capabilities, like mod/quiz:viewreports and mod/quiz:grade (to use the old names) outside of the reports. That says to me that they are nor really report capabilities. It is easier to understand if they remain quiz capabilities. (However, splitting mod/quiz:grade into two mod/quiz:grade = Able to manually grade individual questions and mod/quiz:regrade = Able to do regrades is a good idea.)
3. I can't think of any convincing case where you would want to allow access to the overview report but not the responses report, or vice-versa, so just having one capability mod/quiz:viewreports for both (and for reviewing other people's attempts elsewhere in the quiz code) makes sense to me.
4. For any developer who just wants to knock together a simple customised quiz report, it would be nice if they did not have to worry about defining a capability unless they really needed to.
I know you have done what I originally specified, but I have now changed my mind about how this should work. Sorry.
There are some use-cases that my original proposal was supposed to address, and which I still think we should do something about. They are:
A. Someone wants a make a quiz report plugin showing some information to people who do not have the mod/quiz:viewreports capability. For example, a helpdesk report that lets help desk staff see just the times when each user submitted their last attempt, for an organisation where quiz grades and responses are considered highly confidential.
B. Someone wants a restricted report, so that even if you have 'mod/quiz:viewreports' you may not see this restricted report. For example, a University has a lot of teachers that are confused by the statistics report and this is costing a fortune in calls to the help desk. Therefore, the managers just want to hide the statistics report from ordinary teachers.
C. Someone wants to make a report with some sub-functionality controlled by a separate capability. A (not very good) example, in the helpdesk report from A, should help desk staff see the number of attempts of each user, in addition to the time of their most recent attempt?
I would really like to meet these needs while introducing the minimal amount of extra complexity. Here is my proposal:
C is easy to cope with, providing reports can define capabilities if they need to. Then the report developer can use whatever has_capability calls they like in the code. We just need the bit that makes these capabilities available for overriding.
A. and B. need a bit more, because we need to control which tabs are visible (as efficiently as possible!). My suggestion now is that we add a requiredcapability column to the quiz_report table that defaults to mod/quiz:viewreports, and that this capability is used to control whether the report appears in the tab bar, and in the require_capability call in mod/quiz/report.php.
Hmm, I can't see right now where in the install/upgrade code the quiz_report table is initialised. Therefore I can't see how hard it would be to let each report specify the capability it wants to require at install time.
And, in a default install of Moodle, I would like to see just the following list of capabilities that affect the reports
mod/quiz:viewreports
mod/quiz:grade
mod/quiz:regrade
quizreport/statistics:view
and the requiredcapability for the standard reports would be
Grades - mod/quiz:viewreports
Results - mod/quiz:viewreports
Statistics - quizreport/statistics:view
Manual grading - mod/quiz:grade
Comments?
There is now a new function that allows you to list capabilities to override at the module level other than mod/{modulename}:{capname}.
The new function is called {modulename}_get_extra_capabilities
Currently we seem to agree that capabilities should be defined in report/{reportname}/db/access.php file. And that each report should have a capability name quizreport/{reportname}:view
Unfortunately, in the UI to override the capabilities, the capabilities will likely appear under different headings to each other and different headings to the quiz capabilities.
Not quite sure how we want to handle capabilities for grade and regrade now. Regrade is now not a separate report so there will be no quizreport/regrade:view capability. You said previously that you thought quizreport/grading:view should also control manual grading capabilities elsewhere across the quiz.