Set definition file handling improvements. #2194
Conversation
drgrice1
force-pushed
the
setdef-rework
branch
from
7d0cc85
to
7578405
Compare
The set definition utility methods in ProblemSetList.pm have been moved into a separate file. The new file is `lib/WeBWorK/File/SetDef.pm`. The methods in this file do the same things that the previous methods in `lib/WeBWorK/ContentGenerator/Instructor/ProblemSetList.pm` with some improvements. This separates the set definition file handling from the content generator user interface code. The `readSetDef` no long returns a reference to a messy array of disorganized data. Instead it returns a reference to a hash whose keys give information as to what the data fields are. Using this hash in the method itself also facilitates a much cleaner way to extract the data from a set definition file than the massive `if .. elsif ..` mess from before. Note that if dates are out of order, instead of just leaving them and warning about it, the defaults relative to the due date are used just as they would be when creating a new set. If the due date is not defined, then one week from the current time be used. The messages about these dates being out of order in the file are still shown though. Another big improvement is that `warn` is never used in the file. Instead any of those previous warnings are returned in a reference to an array. Each element of the array is also an array reference whose contents are suitable for passing to maketext. This is one of the needed steps toward removing the global warning handler. To accommodate more complex warning messages the `addgoodmessage` and `addbadmessage` methods need to be placed in a `div` container with the "alert" role instead of a `p`. Visibly this makes no difference. Using this the warnings from before are now in the alert messages returned from the SetDef.pm methods.
drgrice1
force-pushed
the
setdef-rework
branch
from
7578405
to
27c0d4c
Compare
This makes it so that the first thirty files in both lists are shown. If there are more than thirty, then the last item in the list will say that there are more files not shown. To make this work a `min` method was added to Utils.pm (there was a max method, but no min method?), and the change in openwebwork#2194 to make good and bad messages be in a `div` instead of a `p` was added here too.
This makes it so that the first thirty files in both lists are shown. If there are more than thirty, then the last item in the list will say that there are more files not shown. To make this work a `min` method was added to Utils.pm (there was a max method, but no min method?), and the change in openwebwork#2194 to make good and bad messages be in a `div` instead of a `p` was added here too.
|
This PR broke something. On develop, if I import a set def file that is numbered other than consecutively from 1 (say 1, 3, 5, ...) then following this PR, it gets imported as 1, 2, 3, ... anyway. I used git bisect to trace it to 27c0d4c. It's reasonable to have set def files numbered some way like this, if the exercises correspond to textbook exercises you have selected to assign and you want to maintain the numbering connection. |
|
There is something weird that could maybe be addressed at the same time as what I just posted about. I had a set def file numbered 1,2,3,4,5,6,7,8. While testing (on 2.18) I changed it to 0,2,3,4,5,6,7,8 and it imports successfully, showing numbers 1,2,3,4,5,6,7,8. Then I changed it to 0,1,2,3,4,5,6,7 and there is an error message when I try to import it. It leaves a set that has one problem. So it seems that the problem numbered 0 in the def file gets built as numbered 1 in the set, and that explains both things that happened. In the second situation, the error happens when a second "problem 1" is being added to the set. |
This shouldn't be hard to fix. First note that The question is how to make that assignment. For example, if you have |
|
My thought is |
|
Negative problem ids (and any problem id that does not consist entirely of digits) are dealt with already when the set definition file is read. So The cases of a set definition containing any invalid problem id comes from someone editing a set definition file by hand. To be frank, if someone creates a malformed set definition file by hand, then we should not be going to extreme lengths to preserve order and such. We just don't want errors. |
The set definition utility methods in ProblemSetList.pm have been moved into a separate file. The new file is
lib/WeBWorK/File/SetDef.pm. The methods in this file do the same things that the previous methods inlib/WeBWorK/ContentGenerator/Instructor/ProblemSetList.pmwith some improvements. This separates the set definition file handling from the content generator user interface code.The
readSetDefno long returns a reference to a messy array of disorganized data. Instead it returns a reference to a hash whose keys give information as to what the data fields are. Using this hash in the method itself also facilitates a much cleaner way to extract the data from a set definition file than the massiveif .. elsif ..mess from before.Note that if dates are out of order, instead of just leaving them and warning about it, the defaults relative to the due date are used just as they would be when creating a new set. If the due date is not defined, then one week from the current time be used. The messages about these dates being out of order in the file are still shown though.
Another big improvement is that
warnis never used in the file. Instead any of those previous warnings are returned in a reference to an array. Each element of the array is also an array reference whose contents are suitable for passing to maketext. This is one of the needed steps toward removing the global warning handler.To accommodate more complex warning messages the
addgoodmessageandaddbadmessagemethods need to be placed in adivcontainer with the "alert" role instead of ap. Visibly this makes no difference. Using this the warnings from before are now in the alert messages returned from the SetDef.pm methods.