Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Conversation

drgrice1
Copy link
Member

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

Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I like the new options for including set definition files in OPL and Contrib.

However, you are ridding us of our NPL legacy. Heresy @drgrice1 . ;)

@drgrice1 drgrice1 force-pushed the sets-manager-set-def-list-improvements branch 2 times, most recently from 9bd188b to f9004a2 Compare April 10, 2024 20:49
@Alex-Jordan
Copy link
Contributor

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
Copy link
Contributor

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

@Alex-Jordan
Copy link
Contributor

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
Copy link
Member Author

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 drgrice1 force-pushed the sets-manager-set-def-list-improvements branch 2 times, most recently from edddea8 to e4e9c52 Compare April 13, 2024 22:02
@drgrice1
Copy link
Member Author

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

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 openwebwork#2361 (and does more).
The default disabled "Select a Set Definition File" needs to always be
present so that if an option is no longer available due to one of the
"List OPL Sets" or "List Contrib Sets" check boxes being unchecked after
selecting one of those set definition files, then the default can be
selected again.
@drgrice1 drgrice1 force-pushed the sets-manager-set-def-list-improvements branch from 2d3b546 to 5bb90cd Compare April 13, 2024 22:40
@drgrice1
Copy link
Member Author

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
Copy link
Contributor

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

@drgrice1
Copy link
Member Author

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.

@Alex-Jordan Alex-Jordan merged commit cf3f225 into openwebwork:develop Apr 13, 2024
2 checks passed
@drgrice1 drgrice1 deleted the sets-manager-set-def-list-improvements branch April 13, 2024 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants