studentquiz / moodle-mod_studentquiz

Moodle-Plugin
GNU General Public License v3.0
38 stars 38 forks source link

Student quiz does not work well with strange Roles setups #64

Closed timhunt closed 5 years ago

timhunt commented 6 years ago

At the Open University, we have lots of custom roles defined, many of which do not follow any of the Moodle 'archetypes'. Because of that, the logic in mod_studentquiz_ensure_question_capabilities does not work for us. I plan to think about that in details, and suggest a fix, but I have not done that yet.

In addition, studentquiz does not implement the studentquiz_get_extra_capabilities function to say that the question bank capabilties are relevant to studentquiz, therefore, it is not currently possible to fix the permissions manually. This bit is easert to sort out, and I have a commit for that already.

timhunt commented 6 years ago

The effect of that change is that when you go to Student quiz admin -> Permissions, you can see and edit what is going on with the question bank permissions.

More on the first half of this issue soon.

frankkoch commented 6 years ago

Thank you Tim for your contribution. We also have some commits. I will wait for your next commit and then its getting time for SQv3.1.2.

timhunt commented 6 years ago

Here is the second commit, which changes the studentquiz_get_extra_capabilities logic.

The implementation I have gone for is, for any role with permission to submit questions in the StudentQuiz, ensure that role has the necessary question permissions. However, it only bothers to add an override if that role does not already have the permissions. I hope that works in all cases.

While looking into this I found all the TODOs in load_questionbank in viewlib.php. I would love to have a go at cleaning that up, but I don't have time today. (Also, if you are about to release a new version, now is not a good time for radical surgury like that.)

frankkoch commented 6 years ago

Thank you again for this contribution. We will publish it in the next release, most likely next week.

If you have time for the TODOs, you're most welcome :-)

frankkoch commented 6 years ago

Hi Tim, there is a problem with commit 147d965. It causes the error "you must select a question type" when a student adds a question. However, teacher can add questions.

timhunt commented 6 years ago

I don't think commit 147d9652b4c5482e4d8d28eafd2d78663bba28a7 could cause this. That just controls what permissions are displayed when you go to Student quiz administration -> Permissions. I think it must be 52541705c09af95571d7d55103dbd8aa8600e6d8, but I can't immediately see why.

I am afraid I am on holiday tomorrow (Friday 2nd November) so I won't be able to investigate this until Monday (5th).

I think there are two things to check: A) when you create a new Student Quiz, are the right permissions set up (which you can now verify using Student quiz administration -> Permissions). If not, the bug is in the mod_studentquiz_ensure_question_capabilities change.

If the permissions are set right, then the problem is likely to be with this bit of the change: https://github.com/timhunt/moodle-mod_studentquiz/commit/52541705c09af95571d7d55103dbd8aa8600e6d8#diff-25eb2cfd41b242f700f7c778c4416c17L841

Sorry about this.

frankkoch commented 6 years ago

Enjoy your long weekend, no hurry.

I cherry-picked 147d965 first and that's when the error came up. Actually, the error also shows when a teacher wants to add a question after a fresh login. The error disappears when he tries a 2nd time. In case of a student the error persists.

The permission to submit questions on StudentQuiz is granted to {Student, Non-editing teacher, Teacher, Manager}.

timhunt commented 5 years ago

I have not yet been able to reproduce this problem. I also extended the Behat test to do what I think you described, and that passes.

Can you tell me more about the scenario that breaks? Thanks, and sorry for the regression.

frankkoch commented 5 years ago

I just tried to reproduce the scenario and this time I figured out that the error occurs even before I cherry-picked 147d965. So it looks like this error is not in your code but in the latest commits of our develop4-branch. It's tricky, because the error happens just "sometimes". So in my first tests, it appeared just after I installed picked 147d965. This time, it came up before.

Sorry Tim, it's obviously not your code which causes the error. We have to reconsider the last commits in the develop4-branch.

timhunt commented 5 years ago

No worries. I think the improvements to the Behat test are a good thing anyway :-)

There is even a chance that teh cahnges in my second commit might solve the problem? (I'm not sure what the problem is though.)

frankkoch commented 5 years ago

To describe the problem: when a student does "Create new question" he gets: capture

However, when you try a 2nd, 3rd, 4th, .. time, eventually the "Choose a question type to add" window pops up. This is without any of your commits. It sometimes even happens with the teacher role.

frankkoch commented 5 years ago

Ah, just to add, it happens with our latest develop4-branch.

timhunt commented 5 years ago

If that is happening then what it means is that the JavaScript that handles that 'Create new question' button has not initialised before you click the button. (So it does a normal form submit to that 'Choose a question type to add' page.) So, open your browser console while the page loads, to see if there are any error messages.

This is standard Moodle javacript (question/yui/src/chooser/js/chooser.js) and I have not heard of it failing before, but if need be try putting a break point in initializer there.

I just tried with your develop4 branch, but I could not make the problem happen on my computer.

On the 'You must select a question type' page, it should show the list of question types, so you can select on there and then click continue. Does it do that? (I suppose the other issue might be that the list of allowed question types is empty. I am not sure what would happen in that case.)

Not sure if any of that is helpful.

frankkoch commented 5 years ago

released with 3.2.0