Activity

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.

Jamie Pratt
added a comment - 30/Oct/08 6:36 PM 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.

"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. "

We can add an exception to component_level_changed to make the capabilities appear under one heading in the module view. But they don't even appear together in the course view. The capabilities are displayed sorted by real name so mod/quiz is not near quizreport/overview

Jamie Pratt
added a comment - 30/Oct/08 7:56 PM "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. "
We can add an exception to component_level_changed to make the capabilities appear under one heading in the module view. But they don't even appear together in the course view. The capabilities are displayed sorted by real name so mod/quiz is not near quizreport/overview

I think we need to fix this so that these capabilities all appear together under the quiz heading, and after the mod/quiz capabilities - that is definitely the right interface from the user's point of view. However, we have to find a way to achieve that without doing anything too hacky in accesslib.php. I am happy to review any patch you come up with, and suggest ways to make it less hacky.

I think regrading is a quiz feature - it just so happens that the UI is shown in the overview report. It should either be controlled by the manual grading capability (mod/quiz:grade) or perhaps it should be a now capability. Or perhaps it should be linked to mod/quiz:manage, since that is what lets you change the question weights and cause a regrade to be necessary. I just added Phil as a watcher in case he has a view.

Tim Hunt
added a comment - 31/Oct/08 1:13 PM I think we need to fix this so that these capabilities all appear together under the quiz heading, and after the mod/quiz capabilities - that is definitely the right interface from the user's point of view. However, we have to find a way to achieve that without doing anything too hacky in accesslib.php. I am happy to review any patch you come up with, and suggest ways to make it less hacky.
I think regrading is a quiz feature - it just so happens that the UI is shown in the overview report. It should either be controlled by the manual grading capability (mod/quiz:grade) or perhaps it should be a now capability. Or perhaps it should be linked to mod/quiz:manage, since that is what lets you change the question weights and cause a regrade to be necessary. I just added Phil as a watcher in case he has a view.

Jamie Pratt
added a comment - 31/Oct/08 3:26 PM - edited I have attched a patch "permissionschanges.txt " to add and use the new capabilities quizreport/*:view and quizreport/overview:regrade
I have not fixed the UI code for grouping capabilities.

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

Tim Hunt
added a comment - 13/Nov/08 11:56 AM 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?

Jamie Pratt
added a comment - 24/Nov/08 10:18 PM Attaching a new version of permissionchanges2.txt patch that fixes a small issue that the quiz_report_list function was not available from review.php which also has the report tab enabled.

Tim Hunt
added a comment - 25/Nov/08 4:55 PM As far as I can see, on upgrade, you don't insert the right capability for the statistics report into the quiz_report table.
Apart from that, the code looks good. Do you want me to test it as well before you commit it?
Thanks.

Tim Hunt
added a comment - 25/Nov/08 5:10 PM OK, am testing.
I obviously failed to read your code carefully, because the quiz_report table was updated properly.
However, two minor bugs.
1. I don't see the option to override quizreport/statistics:view in the quiz context. (But I was able to override it in the course context.)
2. After I prevent teachers from seeing having quizreport/statistics:view, I can still see the stats report!
Ah, this is just one bug, in quiz_report_list, one of the if statements needs to be changed to
//add any other reports on the end
foreach ($reportdirs as $reportname) {
if (!isset($reportcaps [$reportname] ))
{
$reportcaps[$reportname] = 'mod/quiz:viewreports';
}
}