WeBWorK 2.21 Release Candidate#2940
Open
drgrice1 wants to merge 406 commits into
Open
Conversation
Configure rpc endpoints to disable cookies for specific courses/users.
Add "PG Critic" to the PG problem editor.
enhanced subject line formatting options for email
…e unanswered. When the "Grade Test" button is clicked, JavaScript parses the `probstatus` inputs and determines what problems are in the test. Then it finds the answer inputs in the page and determines if there are any questions in a problem that are unanswered. If there are any problems with unanswered questions, then a message stating that is added to the confirmation dialog. This is to address the feature request in issue #2183. Generally, this will work for most answers. However, there are a couple of cases that don't work right. These are cases where answer inputs are non-empty even if a student has not actively entered an answer. Most notable is the case of an answer to a `draggableProof.pl` or `draggableSubset.pl` question. The JavaScript for those always fills in a non-empty answer as soon as the problem is loaded into the page. Another case is if a problem uses the deprecated `PopUp` method from the `parserPopUp.pl` macro or the deprecated methods from `PGchoicemacros.pl`. For example, if `PopUp([ '?', 'a', 'b', 'c' ], 1)` is used. That will have the non-empty value `?` for the default answer. Unanswered questions that use the `draggableProof.pl` and `draggableSubset.pl` macros could be determined server side, but the only way to do so would be to process the problem via PG. There were several suggestions in issue #2183 to use a server side set "flag" or have other server queries involved in this. Those really don't help other than in this case, and generally shouldn't be done. We really do not want to have a submission to the server that processes all problems in a test to determine if there are unanswered questions before submitting a test for grading in which all problems will again be processed. There was a suggestion to change the "preview test" button into a "save answers" button, but even that doesn't really help. It would not be reliable, because the information would only be there if the button were used first. There is no guarantee it would be used first, and this needs to have the ability to process the answers regardless. Note that attempting to correctly determine if a question that uses `parserPopUp.pl` or `PGchoicemacros.pl` is unanswered cannot even be done by processing the problem. This just adds emphasis that those should not be used anymore. There may be some other cases that have non-empty answers without students actively answering a question, but I can't think of any at this point. One thing to note is that if a problem has never even been displayed on the page (for example a `draggableProof.pl` problem that is on a page of the test the student never goes to) this approach will work correctly even for the above cases.
Add a message to the grade test confirmation dialog when questions are unanswered.
Add message to PG critic output if no issues are found.
This module is broken and I am rather certain it can't even be fixed. It uses an archaic and rather unsecure approach of trying to read cookies from Moodle and using that for authentication. In most cases this would be a cross domain cookie and most likely impossible to read. This approach might have worked a long time ago, but cookie's have changed a lot since then.
Remove the Moodle authentication module.
Remove the `Plots::GD` PG module dependency.
This deletes the course map entry for the course's previous name, and then creates a new entry for the course's new name. Since the `renameCourse` method is also used when unarchiving a course to a different name, care needs to be taken to ensure that this is only done for a course rename, and not for that situation. Thus there is a new `updateLTICourseMap` option for the method. This is only true when the method is called to rename a course directly. This addresses issue #2824.
…link_uri. This is useful if you want to make unpublished instructor links in the LMS that go to specific pages in webwork with query params to make certain display options active immediately. For example, if you use a link such as `https://webwork.server.edu/webwork2/instructor/progress/set/setID?returning=1&show_date=1&show_testtime=1&show_problems=1` then the test dates, times, and problems will be shown when the page opens. Currently URL parameters are dropped. This was not intentional when I implemented this, it is merely a side effect of calling `systemlink` and `url_for` on the `target_link_uri` that is given. This just gets the query parameters that are passed in the given `target_link_uri` and then passes them to `systemLink` so that it adds them back to the generated URL that gets redirected to.
… generation. First, there are some minor tweaks to the POD and sample problem viewers: * Fix the wording on the sample problem viewer index page for "Sample Problems by Macro". It current says, "For many macros, this lists all sample problems used by the macro." However, macros do not use sample problems. Sample problems use macros. So the wording is now, "For many macros, this lists all sample problems that use the macro." * Update the version of bootstrap in the POD templates to 5.3.7 which is currently the latest. * Update the links in the pod templates to wiki.openwebwork.org. * Remove the `favicon` links. Those are not explicitly needed. Browser's now load the favicon even if the page does not contain the link. The link only needs to be added if the file is not named favicon.ico. The following are the actual changes needed to accomodate changes to the PG POD and sample problem generation: * Remove files that are now used from the PG repository. * Remove the PODViewer js and css files, the PODParser.pm and PODtoHTML.pm modules, and category-index.mt and pod.mt templates, all of which are now provided by PG. * Make the necessary changes to the generate-ww-pg-pod.pl script so that it can use those files from the PG repository. * Update the SampleProblemParser module usage for its changes and namespace change. * The `WeBWorK::PG::SampleProblemParser` module now handles generation of the actual search data with its `getSearchData` method. So the `WeBWorK::ContentGenerator::SampleProblemViewer` just calls that method. * The `assets/stop-words-en.txt` file is now in the PG repository so that the `WeBWorK::PG::SampleProblemParser::getSearchData` method can get at it easily. * The `htdocs/js/SampleProblemViewer/documentation-search.js` file is now in the PG repository, and webwork2 uses it from there. PG also copies it to the PG docs github pages site in its new workflow. The options are passed in via the script in `templates/ContentGenerator/SampleProblemViewer.html.ep`.
This make those fields that have the special word values (such as "Unlimited", "Set Default", etc.) be numeric inputs again. However, those special word values are separated into a select. So now there is a select and a number input. The select gives the option to choose one of those word values, and it also has a numeric option (labeled appropriately for the field). When that numeric option is selected then the value in the number input is used. With this change the old "undoLabels" hack is no longer needed. That is the hack that switched the word values back to the numeric values server side. The select options already have the correct value. The select also has a special "numeric" value that signals that the number in the number input is to be used instead. This is an approach to replace the previous number input approach implemented in #2820 and reverted in #2823. Unfortunately that approach had some issues that could not be relegated purely with a number input. I also noticed that there was an issue when the $test{maxProblemsPerPage} variable is set to 1. In that case the `problems_per_page` setting would not be shown when editing the global set or editing a set for several users. However, when editing the set for a single user the setting would be shown, although it still couldn't be edited. It doesn't make any sense to show an option that can't be edited for the set as a whole, and isn't even shown in that case, when editing for a single user. In fixing that issue I noticed that the override "none" setting in the `FIELD_PROPERTIES` hash is rather messed up. See the comment I added on line 84 of the `ProblemSetDetail.pm` file. That setting is no longer used since I removed the `attempted`, `last_answer`, `num_correct`, and `num_incorrect` fields from the hash that were nonsensically included in that hash with the override "none" and type "hidden" values, which basically meant that those fields were ignored everywhere. Note the `FIELD_PROPERTIES_GWQUIZ` constant was also remove because it was not used in actuality. The only field in that hash was the `max_attempts` field, but since it is no included in the `GATEWAY_PROBLEM_FIELD_ORDER` array that hash key was never accessed.
problem render below the problem fields on the set detail page.
set, and the select is switched from a non-numeric value selection to the numeric value selection on the set detail page.
…ts in the sets manager. When editing sets in the sets manager, there are now some new inputs and such above the set list table. There is a select to choose which set date to change (the open date, reduced scoring date, close date, or answer date), and which date to set that to for all selected sets. Obviously the reduced scoring date is not shown if not enabled for the course. Then there is an "Apply to Selected Sets" button which if clicked will set the selected date type to the chosen date for all selected sets. There now needs to be the set check boxes on the edit page (these were not shown in edit mode before). Internally those checks have a different input name so that they don't conflict with the behavior of the usual checks and since they are for a different purpose. There is validation, and if the button is used when the inputs haven't been set or if no sets are selected, then messages are displayed informing the user of what is needed. Note that the usual ordering of dates applies. So if a set date is out of order then the other dates are adjusted just as if that date were edited directly for the particular set. This is how I envision what was requested in https://forums.openwebwork.org/mod/forum/discuss.php?d=8795#p22520. I don't see the suggested way of this working in that forum post as actually being feasible or even really making sense. The suggestion was to add a button that would copy a date to all sets below it, but the order of sets in the list is not a reasonable way of managing this since that order could depend on the set dates to begin with.
Currently the login capability of a user is only ever checked on initial sign in, and never again. So if a user logs in, and then the status or permission level of the user is changed so that the user no longer has the `allow_course_access` behavior or `login` permission level, then the user's current session remains valid, and the user may continue to work in the course (including submitting answers). This changes that so that those things are checked on each request. So, for example, if a user is dropped (status changed to "D"), then the next thing the user tries to do in the course that involves a request to the server will result in the user being logged out. This was reported for the Shibboleth authentication module in issue #2827, but really is an issue for all authentication modules. So this more generally fixes issue #2827 for all authentication modules. This has been tested for all functional authentication modules (i.e., for all but the `CAS` and `Moodle` authentication modules. If the `CAS` module is fixed this should work for that as well. I plan to remove the `Moodle` authentication module in another pull request. Note that this is done in such a way that no new database queries are needed. To make this happen the user record is cached in the `check_user` call, and then can be used any time after that. Future plans are to take this much further. There are many times in the code that the database record for the current user is fetched from the database, and now this cached user record from the current authentication module could directly be used instead.
The format for a filter in the scrolling record lists is `filterName:filtervalue`. When the filters are applied the name and value are slit on the colon. Now if the filter value contains a colon, then the current code splits that value as well, and since only two variables are saved in the return value, the rest goes off into the abyss. This just adds the optional limit to the `split` call so that the colon in the value is not split. This fixes issue #2885.
Add a no-dark-mode extension for MathJax.
Allow colons in filter values.
…ownload. When the "Archive Course" button in the "File Manager" is used, the course archive is generated in a temporary directory in the root of the course directory. That file is then immediately offered for download, and deleted as soon as the request is completed. This is as discussed in issue #2983.
…lem.pm` package. Currently if the `modify_problem_sets` permssion level is set to "ta", then a "ta" user can use the problem editor. However, if the user selects "Open in a new window" with a different seed than is set in the database for that user or has made local edits saved to a temporary file, then the problem will initially render with that seed and temporary file, but if the "Check Answers" button is used the problem reverts to using the user's seed and source file for the problem from the database. This is because the package is inconsistent on the permissions conditions that it uses. When the problem to render is selected in initialization the `modify_problem_sets` permission is used. However, when it is deteremined if the `sourceFilePath` and `problemSeed` parameters can be hidden in the form, it is checked instead that the user has the "professor" permission level. To fix this those checks need to be made consistent. The "professor" permission level check was incorrect, and the `modify_problem_sets` permission should be used instead. This fixes issue #746. Webwork2's oldest open issue!
…r in dark mode. The `bg-white` class is set for the containing panel for the view area in the PG editor. So for the text to be dark so that it works even in dark mode. This is the quick and easy fix. Better probably would be to use a color scheme responsive class such as the `bg-light` class (as is done for course info files). Then in dark mode you would get a dark background with light colored text. However, that would change the background color in light mode. Currently the background is actual white in light mode, and the `bg-light` class is an off white (same as the background of the site navigation).
…anager. Currently the sort methods are called with the ampersand sigil. That implies the argument of `@_` and with recent versions of Perl that causes the warning `Implicit use of @_ in subroutine entry with signatured subroutine is experimental`. This exact same issue was fixed for the accounts manager page in #2607, and this is the same thing exactly and fixed in the same way.
Well, @somiaj has at least. If you have your window size set to exactly the breakpoint (which is 768 pixels), then the site navigation will not open. The `=` needs to be on the other side of the check in `htdocs/js/System/system.js`.
Using `left: -20%` seems to not be working well at small window widths just above the break point. Using `left: 250px` which exactly matches the sidebar width seems to work better. With `left: -20%` the problem/student nav looks like it is moving to the left to fast.
Fix a major bug with the navigation that everyone has certainly seen.
Add a `main` landmark to pages.
run perltidy on defaults.config
Some odds and ends that I have had sitting here for a couple of years now.
Fix a warning due to the way the sort methods are called in the job manager.
Fix contrast when viewing the xml of a hardcopy theme in the PG editor in dark mode.
…vlem-view Make permissions for viewing an edit file consistent within the `Problem.pm` package.
…wnload Make the course archive generated via the file manager an immediate download.
Suggested clean up and minor issue fixes.
add-on conf files
Any package can have an `incompatibleVersions` flag whose value should be a reference to an array of version strings. If the version on the system of a package is one of these versions, then `check_modules.pl` will warn about it. `Mojolicious` now has this set for versions 9.43, 9.44, 9.45, and 9.46. Also remove unneeded quotes on left operands of a fat comma and hash keys. I missed these when they were added in #2893. I left the quotes on the single word package names which also don't need them, but this is consistent with the other package names that do.
…sions Add a case for incompatible Perl package versions in `check_modules.pl`.
Developers will need to install and use this version to match the workflow. Also update the Github workflow to use node 24, and the actions in the workflow to their newest versions.
Note that some of the files in `bin` that do not have the `.pl` extension were also perltidied. That will not be enforced by the workflow, but this still should be done.
This is basically the same fix used for the hardcopy theme xml preview. This just adds the `text-dark` class to the container. Also switch the "Loading..." placeholder that shows while a problem is rendering to using the `bg-secondary-subtle` background instead of the `bg-secondary` background. The `bg-secondary-subtle` background is responsive to the color scheme, and so works for both light or dark mode.
Pgeditor critic dark mode
When editing a problem and the code has a syntax error that prevents perltidy from working, if perltidy is then used it shows the errors from running perltidy. If you then fix those errors and run perltidy again, the toast shows that pertidy was successful, but the errors are still shown in the render window. This pull request makes it so that when perltidy is successfully executed after a previous executing failed, then the problem is re-rendered, thus removing the perltidy errors. This is detected simply by checking if the render area contains an iframe or not. This addresses issue #3012.
Perltidy update
…dy-failure Re-render a problem in the PG problem editor after a perltidy failure.
In general when editing any kind of problem the option to "Append to end of set" is now shown. Even for a new problem template or a sample problem. Furthermore, the option includes a select with which to choose a set for the course. When editing a problem that is in a set, the set the problem is in is selected by default. Otherwise, the default "Select a Set" option is selected. When the form is submitted and this option is selected, then validation occurs to ensure that a set has been chosen. Also, server side the parameter is validated. Note that if the server receives a request that has the radio selected but a target set is not in the parameters, then the file will be saved, but it will not be added to any set. However, this generally will not happen for those using the problem editor. It will only happen for someone that is properly authenticated and with sufficient permissions, that is hacking on parameters. Wnen editing a course info file, set header file, or hardcopy header file, do not show the "Copy auxiliary files" option. That option should only ever be shown for problem files. When editing a set or hardcopy header, don't show the options to "Replace current problem", "Append to end of set", or "Create unattached problem". Those don't make sense at all for a header file. They are not problems. Instead show options to "Set as set header for set" or "Create unattached header file". The option to "Set as set header for set" also includes a select with which to choose a set for the course that the file will be set as the header for. The set that the file is a set header for is selected by default. When a default set or hardcopy header is being edited don't include `opt/webwork/webwork2/pg` in the default save to file name that is shown. When a sample problem is being edited, use the original file name of the sample problem for the default file name to save the file to. This can still be changed by the author, but it doesn't need to be "newProblem.pg" for these. Note that the message that states that "You can change the file path for this problem manually from the Sets Manager page" when the file that is being saved already exists has been removed. That message has been there for a long time, but it doesn't really make sense to state it at this point. The user has chosen a file name, not knowing that a file by that name already exists. The intent was not to use the existing file for the problem. The intent was to save the current content as the chosen file name and use it for the problem. In addition, this message was shown for any file type, and does not apply to set headers at all. IMPORTANT NOTE: The problem editor no longer attempts to use a set version and nothing should try to open it with a set version. The only case where that was done has been removed. That is from the set detail page when editing a set version for a user. Now on that page, that only sends the set without the version. The reason this was done is because doing so in many cases results in an exception being thrown when you try to save or do many of the things in the problem editor. Even when saving doesn't result in an exception, the save doesn't go where you might think it would go. Also, don't try to use the effective user anywhere in the problem editor. That also does not do what you might think. Generally, these were things that were not really thought out. You really shouldn't be editing a problem for a particular user or set version. So the problem editor never tries to change the source file for a user problem. It always works with the global problem. Also there are a few minor changes to a couple of other tabs. These are below. Don't show the "Convert the code to PGML" and "Analyze code with PG Critic" options on the "Code Maintenance" tab when editing a set header. A set header might have antiquated methods such as BEGIN_TEXT/END_TEXT blocks, but I don't think that it is a very good idea to try the PGML conversion on these. Although the PG critic will work on these files, there are many things that the PG critic will report that don't apply at all to set headers, such as having metadata or a needing solution. Not showing "Convert the code to PGML" is perhaps debatable, but not showing "Analyze code with PG Critic" is really not debatable. On the "View/Reload" and "Generate Hardcopy" tabs do not show the options to change the seed when editing a set header. The seed doesn't really apply to set headers. I don't think a set header should ever use the random methods. Note that the primary intent of this pull request is to address issue #2992.
…s considered a template.
Basically, any file that is not in the course's templates directory is
now considered a template, and for these files the "save" tab is not
shown. Note that this uses the `WeBWorK::Utils::Files::path_is_subdir`
method, and so files that are linked to from the course's templates
directory are not considered templates. Although, if those files are not
writable by the server, then the message `The file "[_1]" is protected.
You may use "Save As" to create a new file.` will be shown. So OPL or
Contrib problems will not be considered templates, but if file
permissions are correct on your server, then the message above will be
shown. However, if a file is in a location in `$webworkDirs{valid_symlinks}`
array, and file permissions are correct on your server, then no message
will be shown, and the file can be saved.
Improvements for the "Save As" form in the PG editor.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the release candidate for WeBWorK 2.21. Please re-target any pull requests that you want to get into the release for this branch.