-
-
Notifications
You must be signed in to change notification settings - Fork 164
WeBWorK 2.20 Release Candidate #2721
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
Open
drgrice1
wants to merge
495
commits into
main
Choose a base branch
from
WeBWorK-2.20
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
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
add controls for when grades are sent to the LMS
This was broken in #2485. In that pull request line 16 of `templates/ContentGenerator/ProblemSet.html.ep` was changed from `<p class="mb-0"><%== $c->{invalidSet} %></p>` to `<p class="mb-0"><%= $c->{invalidSet} %></p>` which means that the invalid set message is now HTML escaped. That was necessary as almost all `invalidSet` messages include the set ID taken directly from the URL, and that is a cross-site scripting vulnerability. However, there is one message that does not use the set id from the URL, but does add HTML that needs to not be escaped. That is the message, `You must use your Learning Management System ([_1]) to access this set. Try logging in to the Learning Management System and visiting the set from there.` where the `[_1]` may be the LMS URL. That now displays as `You must use your Learning Management System (<a href="https://myschool.edu/lms/">the LMS</a>) to access this set. Try logging in to the Learning Management System and visiting the set from there.` `<%=` can certainly not be changed back to `<%==` because of the cross-site scripting vulnerability issue. However, there is another way to prevent HTML escaping. That is by using a `Mojo::Bytestream` object. So this message which is the only one that needs to not be HTML escaped (and is safe to do this with) is set in that way via the `b` method of a `Mojolicious::Controller`.
This commit just makes the changes in the scripts, but does not yet run perltidy with the new setup. That will be done in the next commit. Note that the `-vxl='q'` option has been removed. This option disables vertical alignment of qw quotes. Alignment of qw quotes was added in version 20220613 (the previous version), and this option to disable that was added to the webwork2 (and pg) perltidy configuration to avoid the alignment because it changes so many files. However, I don't think that alignment is bad, and I am tired of adding new options to prevent code changes. With this newer version another option would be needed to prevent the new signed number alignment (although that affects PG much more than webwork2). Note that the `bin/dev_scripts/run-perltidy.pl` script now insists on this specific version of perltidy instead of this version or newer. This is because every time a new version of perltidy comes out, it changes things. So developers really need to be using the same version or it will not be consistent with the github workflow result. Also, the version requirement in the check_modules.pl script has been removed. Only developers need a specific version of perltidy. Those running webwork2 can use other versions. This is only used for formatting problems in the PG problem editor, and using other versions may result in slightly different formatting there, but that is not so consequential that we need to require a specific version. Note that this change depends on the corresponding pg pull request, because in that pull request the `-vxl='q'` option has been removed. Otherwise version 20220613 or newer of perltidy would still be needed.
Fix the invalid set "You must use your LMS to access this set" message.
Switch to using Perl::Tidy version 20240903 (the latest).
When able to grade a test of another user, change the "Grade Test" button to "Grade Test for UserID" to make it clear the test is being graded for another user. Show the "Check Test" button along side the "Grade Test" button for instructor who can use the problem grader but also have the permission to submit the past due test. Allow instructors who can use the problem grader to grade unsubmitted tests for that user without needing the permission to submit test versions or answers for another user. This way if for some reason a student's test doesn't get submitted correctly (such as a user closing their browser without grading the test or after reaching a grade proctor screen), it can still be graded with their saved answers. Update the timer when acting as another user if it is being shown, just only suppress warnings about running out of time in this case. Update description of record_answers_when_acting_as_student to state that it can also start and grade test versions. Also include a warning about that permission should be disabled (set back to "nobody") after using it as it can interfere with tests, and should probably only be used temporarily.
Don't show the quiz timer for instructors who can submit student answers after the due date. Update the help language to try to better describe how dangerous it is to leave the record_answers_when_acting_as_student permission on.
Viewing an open gateway test while acting as a student with the permission to submit answers for that student is dangerous since the user's answers will be saved over the student's answers. In this case, give a warning to the user about the danger and suggest they disable the permission to submit answers as students before viewing the open test version, unless they plan to submit answers for that student. The warning will only appear when they first view the test version unless they back out of the test version loosing the hidden `submit_for_student_ok` parameter.
This adds a long-text `<desc>` tag the histogram images on the stats which fixes the issue mentioned in #2598. In addition this fixes some issues I found with the stats page. * Links on the problem bars weren't working so forced them to be a string, along with wrapping the jitar status bar inside the anchor tag to the corresponding problem. * JitarSets - The adjusted status bars were reversed and didn't need to be and the adjusted scores were multiplet by 100 and didn't need to be.
This splits up the macros on the PODViwer index page to be grouped by their location in the `pg/macros` directory. In addition macros are listed before the libraries. The two macros not in a subdirectory, `PG.pl` and `PGcourse.pl`, are grouped with the pod in `pg/doc`. This makes finding the POD for a specific macro easier. Also update `generate-ww-pg-pd.pl` to match the changes for the POD included on the webwork wiki.
To keep from having to update the list each time a macro directory is added/removed/changed, just sort the macros by their directory alphabetically. There is still a hash of macro names to allow both translated or custom names instead of using the directory as its name, but any directory that isn't in the list will also be included without updating the list.
Add alt text to histograms on stats page.
When acting as another user with permissions to create or record answers as that user, rework how the confirmation message is stored. Instead of piggy backing off of the `invalidSet` key, use a new `actingConformation` key for this case. This also skips deleting any proctor authorization key while waiting for user confirmation to continue.
Store the confirmation state when creating or viewing an open test version for another user with appropriate permissions in the session. This is used to confirm prior proctor authentication and not ask for a second after user confirmation. Credit to drgrice1.
Run `npx update-browserslist-db@latest` since `caniuse-lite` is complaining that it is outdated. Run `npm audit fix` because there is a low severity security vulnerability in the nanoid package (used indirectly by something else we use).
Split up macros on PODViewer index page.
Maintenance of npm packages.
The spans are simply added where needed. No javascript is needed for them to be in the page. The "?" help label is changed to just "Help" in most cases. The "?" is usually not read by screen readers. Note that these all go through the `helpMacro` method and a help file template is rendered. The template can also set the ariaLabel stash value to override the default "Help" text. Most pages shouldn't use the override though (I think). For example, on the "Accounts Manager" page the screen reader says something like "accounts manager heading level 1 link help link" (at least that is what Gnome Orca says for this). I don't think it should be read as "account manager heading level 1 link accounts manager help link". It increases the verbosity unnecessarily. However, on the "Assignments" page the title of the page is the course title. So the default aria label is overridden with "Assignments Help", and the screen reader says something like "course title heading level 1 link assignments help link". Note the help title in the dialog was also changed to "Assigments Help". It still said "Course Home Help". It seems we missed this when the page name was changed. Of course mileage will vary as to what different screen readers say on different systems and with different browsers.
the help content.
setting or variable.
This was an oversight when I made the switch to the PG CodeMirror editor with CodeMirror 6. The sample problems also use CodeMirror for syntax highlighting. Since the CodeMirror 5 javascript is no longer available that fails. The pg-codemirror-editor package now exports a `runMode` method (available via the global `PGCodeMirrorEditor` object), that essentially does what the CodeMirror 5 `runMode` addon method did. It is slightly different in that it only works for the PG language. At this point we don't use it for anything else, so that should be good for now.
This adds a leaderboard for achievements, which ranks users from the greatest to the least number of achievement points along with showing the badges of all earned achievements. The default use of this is to provide a summary page for professors to see how many achievement points students have earned along with which badges they have earned. The default permission level to view the leaderboard and see usernames on the leaderboard is professor. The permission level for viewing the leaderboard and viewing names on the leaderboard can be changed under course configuration to allow students to see the leaderboard. It is noted that since achievement points are often closely related to grades, that this should be considered before allowing students access.
Rename "Leaderboard" to "AchievementsLeaderboard". Implement the code and database improvements to make the page load faster.
Remove old files (notes and problems) from the `/doc` directory
This was removed when the uploads were reworked, but I see now that it should not have been. Without this a warning about the empty string not being a valid upload occurs if there is not an upload.
Add back the "real" upload check.
move helper tool buttons to above problem list
Any time an achievement is created a warning is now displayed. This is due to an incorrect check for `$@` without a preceeding `eval`. This pull request adds the appropriate evals and corresponding checks for exceptions in `$@` which removes the warnings. Also, the delete handler was not correctly maintaining the `visibleAchievementIDs` list. As such, if you create an achievement, delete it, and then create an achievement with the same achievement ID again, two copies of the achievement are shown in the list (although only one actually exists in the datbase). So the delete handler now correctly maintains the `visibleAchievementIDs` list.
…sues Fix an inconsequential warning when a new achievement is created.
ability to cap the number of problems per page in a test
give hints about password fallback
…ated course.conf file
This method is not used, and definitely never should be. The method was intended to be used in a maketext call with `[tense,_1,present]` or `[tense,_1]`. If `present` is given then 'ing' is appended to the given interpolated value, otherwise 'ed' is appended. However, there is no possible way that will be valid for most languages other than English, and hence this was entirely invalid to use and should not exist. This method is a carry over from the original code here which was copied from the `Locale::Maketext::Simple` package.
…problem editor. This ensures that the given file name has the same valid characters that the file manager requires. Show the message "The text box now contains the source of the original problem. You can recover lost edits by using the Back button on your browser." any time that a save is not going to be done in the save_as handler, since it applies to any of those cases. Only set the `editFilePath` when a save really will occur. Finally, don't show the "File "[_1]" exists. File not saved. No changes have been made." message if no file name is given. This fixes issue #2754.
…comma fix. This fixes issue #2739. Prior to this pull request something like `my $a = { y => 1, b => 2, c => 3 };` would have syntax highlighting issues because it was being recognized as the transliteration `y=>1,b=>2,c=` with the quote like delimiters being the equality symbol. This is not exclusive to the `y` transliteration operator. The same could happen with any of the quote like operators (`q`, `qq`, `tr`, etc.). For example, `my $a = { q => 1, b => 3 };` had the same problem. It would be recognized as the single quoted string `q=>1,b=` with the equality symbol again being the quote like start and end delimiter. Of course with this pull request the `=>` is properly recognized as a fat comma.
bring the courses.dist/modelCourse course.conf file closer to a gener…
Remove the localization `tense` method.
…ilename Check the file name for valid characters when using "Save As" in the problem editor.
Update the pg codemirror editor dependency to pull in quote like fat comma fix.
…grading. This finishes @Alex-Jordan's work in #2750, and checks for missing symbolic links in a course's templates directory when checking if courses need upgrading, and creates the missing links when upgrading the course. If a symbolic link points to an incorrect location (as defined in the course environment), then the upgrade process attempts to delete the symbolic link and recreate it pointing to the correct location. Note that the check for permissions of the symbolic links has been removed. For symbolic links there are two permissions in consideration. Those of the link itself, and those of the link target. The permissions of the link itself cannot be changed. Even the linux command line `chmod` utility cannot do this (contrary to what Google AI wants to tell you, there is no `-h` option for `chmod`). Furthermore, linux completely ignores symbolic link permissions, and doesn't use them at all. So the only permissions that really matter are those of the link target which linux does use. However, the directories that are the target of these links do not belong to a particular course. They are system directories (or part of the webwork2 installation), and setting the correct permissions on those is part of the installation of webwork2. It should not be part of a course upgrade process. Including these permissions in the upgrade report when not correct would actually mean that every course would show that it has the incorrect permission, or no courses would. Upgrading would never fix this, because the permissions for these directories cannot be changed from the user interface in any case. What could be done here is to make another admin page that does a system status check, and lets the admin user know what directory permissions in the webwork installation are not correct what needs to be fixed. Of course the `setfilepermissions` script can be used to fix these permissions (it already handles most of the directories in question).
Also fix the logic both here and when archiving a course for the possible combinations of the directories, links, and database needing repair. With the previous logic if the directories were good, but the database tables and links both needed repair, then the button said "Attempt to upgrade directories and links". However, in actuality if that button were used, the server would both repair links and repair database tables. Now, anytime the database tables need repair the button says "Upgrade Course Tables" regardless of the state of the directories and links. This is not ideal since directories or links might also be repaired in this case, but the important thing to show is the database repair which is more important than directories and vastly more important than links.
The OPL and Contrib no longer contain any set definition files, so there is no point in this code. It doesn't do anything anymore. This means the checkboxes shown on the "Sets Manager" import tab and the "Set Definition Files" page of the "Library Browser" are gone. Also, the script to search for set definition files in the OPL and Contrib is removed. All of the places where that script was called are deleted. The code utilizing the data in the set definition JSON files that were generated is gone.
Remove the code for OPL and Contrib set definition files.
Create missing symbolic links in a course templates directory when upgrading.
Move LTI login JavaScript out of html template
This PR has two related features 1. A script in `dev_scripts` to parse all of the sample problems and the POD to build up a "database" of non-common words for a given problem/POD page. 2. Add search boxes to the sample-problems/POD pages to return pages that match the search criteria. Parse the POD from macro files . improve the search and add a "no results" Also add ww-update script to call OPL-update and build-search-json.pl
This is a rework of #2733. This does not add any scripts that need to be run when webwork is installed or upgraded. Instead the search data is dynamically generated. The first time that the sample problems home page is loaded all PG macros and sample problems are parsed and the search data extracted. Then the data is saved to the `DATA/sample-problem-search-data.json` file. This takes less than half a second on my computer, but that will probably vary some depending on the server's processing capabilities. The last modified time of each file is also saved in that search data. On subsequent requests the last modified time is checked, and a file is only parsed again if it has been modified since the last time the page was loaded. If a file is deleted from PG, the data is removed from the file. So even in development the data will always reflect the current state of the PG repository on the server. Subsequent requests when no changes to PG occur usually complete in around 1/25th of a second. A change of a single file increases that time only slightly. Note that the search data is accessed by JavaScript via the new `/sampleproblems/search_data` route. The layout of the sample problems home page is changed considerably from #2733. The layout now works on narrow screens and some accessibility and html validation issues were addressed. There is also quite a bit of improvement in the search data that is saved. There is no attempt to single out methods or functions for macros from the POD headers. Unfortunately, that was never really going to work. Instead the words from all headers (that are not stop words) are used. So the methods or functions that are in a POD header will be in the search data, but other things will also be there. There are some custom stop words added that are not desirable in the search data, like "description", "synopsis", "podlink", and "problink". Also, macros that don't have POD in them are not indexed at all since there isn't anything to show for these files.
Rework the search box for the sample problems page.
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.
It is time for this. We have talked about just using the develop branch, but I think it is nice to have a separate branch for the release candidate. It just keeps things separate in a nice way.
So as usual, re-target pull requests for this branch that you want to get into the release, and I will handle synchronizing those to develop as they are merged.