Update permissions for viewing info on ProblemSet page.

[somiaj]

Rework when a user is able to see set information on the ProblemSet page. This includes being able to see the set description and information header before a set is open.

The goal is to make the ProblemSet page friendlier when a student accesses it from an LMS before the open date or if the set is not currently visible. In addition the set headers can be used to provide information to students before the set opens.

This is done by adding a new permission view_unoppend_set_info which defaults to guest (this allows instructors to change this to ta to get the old behavior). Users with this permission will be shown a ProblemSet page that includes when the set opens along with the set information header and a button to email the instructor before the open date. In addition users with this permission will be shown a warning alert instead of a danger alert if they try to access a hidden set (the set header will not be included in this case).

Note, that the permission view_unoppend_sets description in the course configuration is just being able to view problems on sets which are not open, so this new permission is consistent with the previous description, it just separates seeing set info from seeing set problems.

Care is taken to ensure that students can view (thus access) any existing test versions for tests that have ip restrictions, or if the template open date was changed.

Last, only show the right info panel div when the panel is not empty and doesn't include only whitespace. And translations were added to messages about IP restrictions.

[drgrice1]

This should be

Suggested change

	% if ($info !~ /^\s*$/) {
	% if ($info =~ /\S/) {

That is the usual check that a string contains non white space characters and is a simpler regular expression. Benchmarking shows it is generally faster too!

[drgrice1]

Actually, this change is not right. The problem is that if this actually happens, then the result is an improperly formatted page. The check on line 135 above and the case that the info is actually shown really should match up perfectly, and this is not doing that.

What case are you trying to catch here and avoid showing an empty info? The first thing to check is if it really is a case to be concerned about. If so, then there are better ways to do this.

[somiaj]

I missed the other check that made used the col-8 vs col-12 style above, but I didn't think it mattered, is it really not formatted correctly if the divs don't use all the columns in the grid? Anyways, if you go to a bad set or a set with an error, https://server/webwork2/CourseName/BadSetName, there is an empty gray div floating to the right with nothing in it. I was trying to prevent that from happening. I am unsure if any other pages that use this info div have similar issues where it is just empty.

[drgrice1]

In my opinion yes. Generally, the info box display needs work regardless of this pull request. Not only for this case, but for others where the content may be empty. For example, if your course has an empty course_info.txt file, then that also currently results in the empty gray box. These cases should all not show the info box, and the page should be properly formatted as intended for the layout.

It seems that to make this work right, all modules that provide the info box and such that the info box might be empty should also override the can method (as the ProblemSets.pm module already does), and that method should not only return 0 for permissions reasons but for the empty info box case.

Note that there is another case where $c->can('info') is called that affects the layout. That is in the go method of the base ContentGenerator module (line 113). That sets the layout for the footer to match the main content (note that some modules override that setting later also). So that also needs to choose the correct layout setting for these cases.

There is an efficiency issue with the current setup. The ProblemSets.pm is already performing extra file access due to this approach. The can method is called three times and each time it opens the course info file and reads its contents and compares it to the default. Then the info method opens that file yet again and reads the contents to actually display (assuming the can method returned true). So to do this well with the current setup, the can method really should do all of the work and stash the content, but with protections to ensure it doesn't execute inefficiency multiple times, and the info method should just use the stashed result. Care also needs to be taken for the ProblemSet.pm module, because that doesn't know if it has content until after the initialize method runs (that is when the set header is currently rendered), and that runs after the current can call in the go method of the base ContentGenerator module.

To simplify all of this I think that really we should just do away with the info box entirely. Students really should just be left in the dark anyway, and don't really need or even want instructions! 😈

[somiaj]

Yea, the fact that $c->can('info') is called before $c->initialize does make things harder to just override the can method. Any way that call in the go method could be moved until after the initialize method? I found something that appears to work, I'll just update the setting for the footerWidthClass on a future can call which appears to work. Patch incoming.

[drgrice1]

The call in the go method could be moved after the initialize method. It would just need to use //= instead of =.

[somiaj]

Any preference to moving the method vs updating the value of footerWidthClass at the end of the initialize method as I am currently doing?

[drgrice1]

If you are just updating it with the same value that the go method assignment uses based on the can call result, then I would prefer moving the go method assignment.

[somiaj]

Updated, the method has now been moved.