If you configure a questionnaire with the "students can view responses" control set to something other than "never", and give students the "readallresponses" capability, students still cannot view responses.

This seems to have been introduced as the module was migrated from 1.9 to 2.0. The developer has correctly realised that there was no protection on the report.php file itself and has copied the code in tabs.php so that users without correct capabilities cannot directly access the report page.

However, I think that it has been done slightly wrong. In tabs.php the program flow is if (can readallresponsesanytime) then output stuff elseif (can viewresponses and conditions met) output stuff. However the code in report.php is if (cannot viewresponses & conditions met) error; if (not readallresponsesanytime) error.

Error stops the program flow, so the second error fires even if you have passed the first condition. I think instead we need to merge the two tests into one, like this...

/// If you can't view the questionnaire, or can't view a specified response, error out.
$context = get_context_instance(CONTEXT_MODULE, $cm->id);
if (!has_capability('mod/questionnaire:readallresponseanytime',$context) &&
(!($questionnaire->capabilities->view && $questionnaire->can_view_response($rid))))

{ /// Should never happen, unless called directly by a snoop... print_error('nopermissions', 'moodle', $CFG->wwwroot.'/mod/questionnaire/view.php?id='.$cm->id); }

I'd like a second opinion on this, and then I'm happy to commit if you're happy to accept it?