openwebwork / webwork2

Course management front end for WeBWorK
http://webwork.maa.org/wiki/Main_Page
Other
141 stars 164 forks source link

Rework how set definition files from OPL and Contrib are shown. #2386

Closed drgrice1 closed 2 months ago

drgrice1 commented 3 months ago

First, the WeBWorK::Utils::Instructor::getDefList method no longer searches in the capaLibrary directory (if that link exists in the course's templates directory). Those set definition files are in Contrib, and so are already listed when the set definition files in Contrib are listed. That method also now sorts first by library, then depth, and then upper case set name. The library order is first local set definitions (anything not in Library or Contrib), then the one set in Library, and then the sets in Contrib. The method also does not return the Library or Contrib set definitions at all unless the Library or Contrib links exist and are readable in the course's templates directory.

Next, the useOPLsetDefs and useContribSetDefs options in defaults.conf and localOverrides.conf.dist have been removed, and are no longer honored. Instead there are now check boxes in the import form on the "Sets Manager" page and when viewing set definition files in the "Library Browser" that when checked include the set definition files from OPL or Contrib. The check boxes are not checked by default. These check boxes are not shown if the Library or Contrib links do not exist or are not readable.

The "Include OPL" and "Include Contrib" check boxes that determine if OPL or Contrib problems are included when listing problems on the "Open Problem Library" page of the library browser were moved from the "View Problems" line below to below the buttons on the right, and are only shown when browsing OPL problems, and not for all of the other buttons (like "Course Files", "Course Sets", "Set Definitions Files", etc.) for which those check boxes do not apply.

This fixes issue #2361 (and does more).

Alex-Jordan commented 2 months ago

When I tried this and check the box to show OPL defs, I only have this one set def file appear:

/opt/webwork/libraries/webwork-open-problem-library/OpenProblemLibrary/Michigan/gateways/precalentr2/topic_frac_exp_powers/settopic_frac_exp_powers.def

So it appears there is only one such file (verified with a find at the command line).

But that set def file uses relative paths that don't go through the OPL. It has:

problemList=
topic_frac_exp_powers/prob1.pg
...

So first of all, that's an OPL issue. This set def should either not be there or use Library/... paths.

But a webwork2 issue is that if I click to "View Problems" from that set, I get an ugly error screen instead of something graceful. Let me know if that's an issue to tie into this PR, or if not I can open an issue about it.

Alex-Jordan commented 2 months ago

BTW, it seems that Contrib/Michigan/gateways/... set def files have the same issue.

Alex-Jordan commented 2 months ago

I am noticing something with this PR. Check the List Contrib Sets box, then select some set you can view. IT could be one of the CAPA sets. Click to View Problems. When the problems appear, the " Browse from: " select menu moves to the item at the bottom of the list. It does not stay at the set you selected to view.

drgrice1 commented 2 months ago

Yes, there is only one set definition file in the OPL. It seems the issue with viewing problems from that set definition file also occurs with webwork 2.18, so that isn't new.

I will look into the issue with the select menu.

drgrice1 commented 2 months ago

The issue with the selection not being preserved is fixed now.

drgrice1 commented 2 months ago

The one OPL set definition file would actually work in the library browser if it were one directory higher than where it currently is. Currently the file is located in the directory /opt/webwork/libraries/webwork-open-problem-library/OpenProblemLibrary/Michigan/gateways/precalentr2/topic_frac_exp_powers. It it were in the directory /opt/webwork/libraries/webwork-open-problem-library/OpenProblemLibrary/Michigan/gateways/precalentr2 it would work. Either that or if the problems in the set definition file didn't have the directory topic_frac_exp_power in their file names it would also work.

There is a munge_pg_file_path method in SetMaker.pm that allows the file names in the set definition to be relative to the location of the the set definition file. This is in fact what makes most of the CAPA sets work since the problem file names in those set definitions are relative to the location of the set definition files.

However, none of those will work if these sets are actually imported in the sets manager since it doesn't have a special case allowing for file names in set definition files to be relative the set definition file location. In fact it doesn't even save the set definition file a set is imported from, so how could it do that.

I observed this when I was working on this pull request, and it seems to me that these set definition files should be either removed, or updated to have the Contrib or Library path in them, and the munge_pg_file_path method should not allow for file names to be relative to the set definition file location.

Alex-Jordan commented 2 months ago

I'm good with this and can merge it. Any reason not to at this point?

drgrice1 commented 2 months ago

No reason that I know of. I will probably fix the issue with the library browser dying when a file in a set definition file is not found, but will do that in a separate pull request.