WeBWorK 2.21 Release Candidate#2940
Open
drgrice1 wants to merge 348 commits into
Open
Conversation
… page. This is a quick fix that will make things function for now, and is good for a hotfix. This just reverts all of the fields that were switched to number inputs and that have labels for special negative values back to text inputs and the functionality prior to WeBWorK 2.20. The fields that do not have those labels can still be number inputs. That will work fine for those. Also set the default for showMeAnother to the "Course Default" instead of "Never" which is what it should be. This resolves issue #2820 for the current release. We can try to do better for the next release, but I don't see any other easy way to fix this that is appropriate for a hotfix.
…ber-settings Revert number inputs for fields with labels on the problem set detail page.
Fix many typos in the code.
If `only_full_group_by` is included in the `sql_mode` mysql settings, then attempting to view problems in the library browser results in an exception with a message like `DBD::MariaDB::db selectall_arrayref failed: 'webwork.pgf.morelt_id' isn't in GROUP BY at /opt/webwork/webwork2/lib/WeBWorK/DB/Database.pm line 137.` This just adds a `MAX` call to the columns of the `OPL_pgfile` table that are selected in the `GROUP BY` call, and eliminates the exception. The resulting data that is returned in the query is the same as before since these columns will all hold the same data for the grouped `filepath` results. This is really just working around poor design in the OPL table structure. This has been noted a few times in the forums recently. See, for instance, the latest posts in https://forums.openwebwork.org/mod/forum/discuss.php?d=8757#p22352. To test this set ``` sql_mode = "ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION" ``` in the `/etc/mysql/conf.d/mysql.cnf` file and execute `sudo systemctl restart msyql`. With the shown value for `sql_mode` and the develop or main branches you will get the exception when you try to view problems in the library browser, but will not with this pull request. Note that the value for `sql_mode` above seems to be the default for MariaDB if `ONLY_FULL_GROUP_BY` is removed. At least that is the case on my system. You can execute `select @@sql_mode` in mysql to see what the settings are for your system. The default for MySQL does include `ONLY_FULL_GROUP_BY` (at least if I am reading things right).
I had the math wrong in #2818. It is correct if the number of problems does not evenly divide the number of problems per page, but if it does then the modulus returns 0. In that case it should also use the number of problems per page.
Fix the math in the test page navigation.
Fix issues when the sql_mode includes the only_full_group_by setting.
The content generator `hidden_authen_fields` method should be called in all page forms, but was not added ot the form in the LTI update page. As a result if you are acting as a student, go to that page, and click "Update Grades", then the acting ceases. Even worse, if `session_management_via` is set to "key", and you click "Update Grades", then you are sent to the login page, and have to login again in order for the form to submit. I missed this when this page was created. @somiaj observed the issue when acting as another user on the page.
First, fix a general issue when a problem fails to render that results in a message that has no relation to the actual problem. The issue is that the `create_ans_str_from_responses` method of the `WeBWorK::Utils::ProblemProcessing` module is called with a `$pg` hash that doesn't contain the right things, and the method is not safe guarded against that. This fixes issue #2829. Second, rework how error handling is done in tests. Instead of just rendering the errors for problems that failed to render, still render all problems on the page. Show the errors for the problems that failed, but still render all of the problems that didn't fail. This means, that the problem navigation is still shown. In fact the entire page renders normally, but the error message is shown for the problems that fail to render. So the student can at least continue to work the rest of the test normally. The student can even grade the test on a page for which a problem fails to render. Note that the "request information" is no longer shown for these messages when in a test. A special `briefErrorOutput` stash value can be set when the `templates/ContentGenerator/Base/error_output.html.ep` template is rendered to skip that. Although, I really thing that information should never be shown in the page for any exception that occurs. I don't think that the time, method, uri, and HTTP headers of the request, ever help with these exceptions.
Add the hidden authentication fields to the LTI update form.
…lure Rework error handling when a problem fails to render in a test.
For the most part, there isn't much that is needed. The javascript
files are no longer in the `es5` subdirectory, and the workaround for
changing the math renderer was removed (as noted in the comment above
it).
The `mathInteraction.pg` problem in the student orientation has been
updated, although further updates may be needed. The screen shots the
problem uses were updated, but someone else might need to create better
screen shots. Mine seem to have come out quite a bit smaller than the
previous ones.
I removed the rather old examples of the `$pg{specialPGEnvironmentVars}{problemPreamble}`
and `$pg{specialPGEnvironmentVars}{problemPostamble}` variables in
`defaults.config` and `localOverrides.conf.dist` that include examples
of doing things with MathJax that weren't even valid for MathJax 3. I
also made the deprecation statement for that variables stronger.
There is a minor css tweak in `system.scss` to ensure that the MathJax
expression eplorer help dialog is above the masthead and stick problem
navigation bar.
Further modification to the MathJax configuration (in
`htdocs/js/MathJaxConfig/mathjax-config.js`) may be needed as per the
discussion in issue #2828. But that can wait until we have more
information.
Update MathJax to version 4.0.
There have been requests to either remove this extension or at least make it so that those editing problems do not have it loaded, as it makes it easier to determine what is wrong with TeX in a problem. This pull request just removes the `noerrors` MathJax extension. So TeX errors are shown for all users. I am not sure why this package was added. It seems that I added it when I upgraded from MathJax version 2 to version 3, but I don't remember why. Perhaps it was just in the configuration that @dpvc recommended, or maybe I added it for some reason. Perhaps it was just about maintaining compatibility with version 2 of MathJax. For version 2 (as I understand it), the `noerrors` extension was included by default, but with MathJax version 3 it must be explicitly loaded. The change from the `webwork_url` to the `webwork_js_config` method in the `WeBWorK::ContentGenerator` module is not needed for this, but should be made anyway. There is also a `webwork_url` method in the `Mojolicious::WeBWorK` module that is already available for all controller modules (since it is a Mojolicious helper method), and having this other one overrides that one and it is confusing to have both that return almost the same value. The only difference is that `WeBWorK::ContentGenerator` method called the `location` helper which returns the empty string if the root URL is '/', and the `webwork_url` helper returns '/' in that case. I don't know what I was thinking creating the `WeBWorK::ContentGenerator` method which was really just an alies for the `location` helper method anyway.
First, this adds the permission `webservice_render_problem` used to determine if a user can render a problem with the WebworkWebservice, instead of using the `proctor_quiz_login` permission for this. Second, this adds an additional permission `webservice_render_source` used to determine if a user can render problems using the problem provided with the request. The use case for this is to allow users which can render problems only using a problem filename, but not by providing the problem's source. These permissions are both set to `login_proctor` to match current behavior and are provided to allow server admins to change which users can render problems. These permissions are not added to the course configuration page as they are permissions that should not be modified by most users, only server admins via `localOverrides.conf` or `course.conf`.
Add permissions to render problems with WebworkWebservice.
Make MathJax show errors for bad TeX. (alternate approach)
…-config-alt Revert "Make MathJax show errors for bad TeX. (alternate approach)"
There have been requests to either remove this extension or at least make it so that those editing problems do not have it loaded, as it makes it easier to determine what is wrong with TeX in a problem. This pull request makes it so that these errors are only shown in the PG problem editor. Actually they are shown any time the renderRPC endpoint is used and the `showMathJaxErrors` parameter is set to a true value, but the only time that webwork2 now does this is in the problem editor. This still includes thhe change from the `webwork_url` to the `webwork_js_config` method in the `WeBWorK::ContentGenerator` module. There is also a `webwork_url` method in the `Mojolicious::WeBWorK` module that is already available for all controller modules (since it is a Mojolicious helper method), and having this other one overrides that one and it is confusing to have both that return almost the same value. The only difference is that `WeBWorK::ContentGenerator` method called the `location` helper which returns the empty string if the root URL is '/', and the `webwork_url` helper returns '/' in that case. I don't know what I was thinking creating the `WeBWorK::ContentGenerator` method which was really just an alies for the `location` helper method anyway.
The `webwork2.sty` file currently creates this link using the `\url` command. That command attempts to detect the link type. In this case that results in a file link. Depending on the PDF viewer used, that results in different behavior. In Firefox's PDF viewer it behaves as if it is not a link at all, but just text. Clicking on it does nothing. In Google Chrome's PDF viewer or Evince it is a link, but clicking on it results in a message being displayed that the file does not exist. So this switches to using the `\href` command instead. That requires that both the link URL and link text explicitly be given, but reliably results in a working link.
Fix the openwebwork.org link in hardcopy.
…browser. There have been several complaints about Contrib problems being listed first in the library browser. Perhaps this will help to quiet those complaints. This is done at the database level, and so should be rather efficient.
The `@openwebwork/pg-codemirror-editor` and its dependency packages `@openwebwork/codemirror-lang-pg`, `codemirror-lang-perl`, and `codemirror-lang-mt` have all had all dependencies updated, and the bracket matching issue reported in openwebwork/codemirror-lang-pg#1 fixed. The resulting changes have also been published. This just updates webwork2's dependency so that those updated packages can be used by webwork2. Note, I did take some liberty by pushing the updates for the `@openwebwork/pg-codemirror-editor` and `@openwebwork/codemirror-lang-perl` packages without going through the pull request process. But with the tiered depencies here that would be tedious to go through the approval process from bottom to top.
…ary browser persistent. The status of the checks is saved to local storage and updated when the page loads. So whatever state they had the last time the page was open is restored. Note that the settings are saved per user id and course. So you can set the checks differently for each course. This was (essentially) asked for in issue #2857.
Currently if `$permissionLevels{login} = 'professor'` and a user signs
in via LTI that would be assigned the role of "student", then webwork2
creates the user and signs the user in. However, on subsequent LTI
logins authentication fails. This refuses to create a user if the
requested role would not have permission to login.
Clean up the error messages some. There is a lot of work left to
do on this. The LTIAdvance.pm module has an extremely poor design for
error handling and messaging to go with those errors. The
LTIAdvantage.pm module is only a tad better (I largely just copied the
poor design of the LTIAdvanced.pm module). The `log_error` key is set
and appended to numerous times, frequently resulting in a long run on
message that doesn't really make sense. Also, there were some of these
errors that were adding "LOGIN FAILED". That was removed because The
`Authen.pm` code always prepends that and that resulted in logs with
"LOGIN FAILED LOGIN FAILED ...".
The `authenticate` method is expected to return either 1 or a message
indicating the failure. Currently it returns either 1 or 0. As a
result the messages that are set in the `authenticate` method go into
the abyss. Those messages should be returned instead of setting
`$self->{error}`. Note that the method can still return 0 if no message
should be set (as in the case of the OAuth token failing to verify for
LTI 1.1).
For LTI 1.3 make sure that the fallback_source_of_username is set before
attempting to use it. Otherwise the claim extraction fails and it
results in a database error later.
Fix a minor issue in the authen_LTI.conf.dist file. The
permissionLevels lines should end with semicolons, not commas.
The option is `extra_ssl_headers` in the `conf/webwork2.mojolicious.yml` file. It works just like the `extra_headers` option, except that the headers are only added to responses to secure requests. This is to address a need to add the `Strict-Transport-Security` header to SSL request responses that was brought up in the forums. See https://forums.openwebwork.org/mod/forum/discuss.php?d=8782#p22468. That header should not be added to non SSL requests.
This is simply the result of executing `npx update-browserslist-db@latest`.
If the total weight of a set is zero, due to either no problems in a set or weights set to zero, these items cause a division by zero not allowing a user to see the associated ProblemSet page. This returns 0 in the can_use methods before dividing by zero in this case, fixing the issue.
The `can_recordAnswers` method of the `WeBWorK::ContentGenerator::GatewayQuiz` package can not call any of the `WeBWorK::ContentGenerator::GatewayQuiz` methods because it is called directly by the `WeBWorK::ContentGenerator::LoginProctor` module without a `WeBWorK::ContentGenerator::GatewayQuiz` object. Attempting to do so will cause an exception. This is a bit annoying as the conditions of the `can_gradeUnsubmittedTest` method (including the conditions of the `can_showProblemGrader` method it calls) must be directly used by the `can_recordAnswers` method. There is no way around that though. This definitely should be considered for a hotfix.
…structor.
When an instructor uses the "Grade Test for" button and the test is on
its final submission, currently the grading occurs, but the test is
still set as a proctored test.
To fix this the condition that determines if the assignment type should
be changed needs to check the `$can{recordAnswers}` value in the case
that the `$userID` and `$effectiveUserID` are different. In that case
`$can{recordAnswers}` is the result of the `can_recordAnswers` method
which will be true if either the user has the
`record_answers_when_acting_as_student` permission or the user can grade
an unsubmitted test.
This fixes issue #2962.
The `GatewayQuiz.pm` module is really so convoluted at this point that
it is really becoming impossible to do anything with, and is in
desperate need of a complete overhaul and rewrite. Any time any new
feature is added to the module or change is made to the module it is
almost impossible to go through all of the possibilities and ensure you
haven't broken something.
This was done for the student nav elsewhere but missed here.
Brackets in a string that are intended to be literal brackets need to be escaped. To test this earn the "Mysterious Package (with Ribbons)" achievement reward, make sure the `templates/achievements/surprise_message.txt` file does NOT exist, and then open a problem set page. With the WeBWorK-2.21 release candidate branch an exception is thrown. This particular bug apparently has been there since 2015.
Fix an invalid localization string in a maketext call.
Fix grading proctored tests that require proctor authorization to do so.
…-test Make sure that a proctored test becomes unproctored when graded by instructor.
Fix the active colors of the selected test or filter in the test nav.
Fix division by zero in {Full,Half}CreditSet Items.
Fix the `change_user_id` script for the case that a user does not have a password.
The colors for this were inline style. Colors cannot be inline anymore since the server cannot detect if the user will have the browser in dark mode or not. So this switches to using Bootstrap alerts instead.
Fix colors on the proctor login page for dark mode. (Bootstrap alert aproach)
Now that the student nav is on every set and problem page there is a need to make the retrieval of all users assigned to the set faster. I noticed in a large class with more than 25,000 users this is quite slow now. It takes about 8 seconds for each page to load. The problem is the query is passing all of the user ids of those assigned to the set to the `getUsersWhere` method. That means that `SQL::Abstract` has to process all of those and compile a rather long sql statement. That is slow. So this breaks the list of user ids into chunks of 500. For the class with more than 25,000 users this drops the page load time to just under 2 seconds. Stil not great, but considerably better. On my production placement exam server that has just over 32,000 users it currently takes more than 35 seconds for each page to load. With this pull request it drops the time to about 8 seconds. Again, not great but considerably better. Clearly the placement server is not as fast as my local computer also. Note that for classes with less than 500 users this won't change anything. Also note that this is only an issue for those that have the student nav shown.
This was suggested by @Alex-Jordan, and does seem to be much better.
This set has nothing of value in it anymore. The problems are really not what we want problem authors to use as a model for new problems. It is time for this set to go.
Remove the `Demo` set.
This was a remnant of my first approach for the Demo set of moving it to `assets/pg` and using a link in the model course, and then rewriting all of the problems using modern techniques. This link should not have made it into #2979.
Remove a symbolic link accidentally added in #2979.
Improve the speed of the database user retrieval for the student nav.
I used the wrong thing in #2975.
Fix the test nav.
Add `$achievementExtensionFactor` setting that is used to configure the length of extensions. This works as a multiplicative factor, by multiplying the base time (either 24 or 48 hours) by the factor. The extension time is always rounded to the nearest hour, and cannot be less than a single hour (two hours for the super extensions). This affects all items that have an extension time. * ExtendDueDate * ExtendDueDateGW * ExtendReducedDate * ReducedCred * RessurectGW * RessurectHW * SuperExtendDueDate * SuperExtendReducedDate
Store the setID of all completed sets in the globalHash when evaluating achievements. This allows achievements to use this data vs just counting the number of completed sets. One use case is being able to exclude optional sets, such as review sets, from some achievements without completely excluding them from all achievements. In addition saving all the setIDs can avoid a double counting completed sets, as there is currently no check to ensure a set is not counted multiple times.
The code and rendering panels are now not only vertically resizable, but are horizontally resizable when the window with is at or above the large breakpoint (992 pixels). Furthermore, resizing does not work with the native browser resize via the css `resize` property. Instead it is controlled with JavaScript. The resize grips (which are much more visible now) can be also be focused with the keyboard and when focused the arrow keys can be used to resize the code and render panels. Note that if `Ctrl` is pressed with an arrow key a 1 pixel resize occurs, and if `Alt` is pressed with an arrow key a 50 pixel resize occurs. Without a modifier key the arrow keys perform a 20 pixel resize. In addition, the dimensions are saved to local storage and persist when the page reloads. Unfortunately there will be some flickering of content as the resize occurs after the page loads. Note that the css `resize` property is actually not supported in all browsers, so this actually makes resizing work for those browsers as well. The browsers that do not support the css `resize` property include Firefox for Android, and Safari on IOS. Yeah, those are for mobile devices, and who edits problems on a mobile device? In any case, this makes the resize grips more evident. The native resize grip is rather small in the lower right corner of the CodeMirror editor panel, and many probably don't even know it is there. Note that the code panel has a minmimum width of 400 pixels, and the rendering panel a minimum width of 300 pixels. This works out so that when the window size is 992 pixels the two panels can't really be resized much or at all (when the site navigation menu width of 250 pixels is taken into account) depending on the browser. But at larger window widths resizing can be done. I thought about making it so that the resizing could go all the way to the right and the rendering panel be resized to a width of 0, but decided against it. If that were done, then the rendering would still be occuring even though you can't see it, and that doesn't seem good. I think that this should at least alleviate the request(s) to hide the rendering panel (which I don't think is really a good idea).
Add extension time factor setting.
Better keep track of which sets are complete in achievements.
Rework resizing of the problem 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
6 participants
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.