openwebwork / webwork2

Course management front end for WeBWorK
http://webwork.maa.org/wiki/Main_Page
Other
141 stars 164 forks source link

PGProblemEditor preview button is not setting AnswerHash isPreview key. #2310

Closed somiaj closed 5 months ago

somiaj commented 5 months ago

The following problem should show the feedback message Preview: Preview My Answers when pushing the preview button. When testing with the PGProblemEditor, $ans->{isPreview} is not being set (or is empty) and the message just states Preview:. This behavior is the same in both develop and 2.18.

DOCUMENT();
loadMacros('PGstandard.pl', 'PGML.pl', 'PGcourse.pl');

$pi = Compute('pi');
$picmp = $pi->cmp(checker => sub {
    my ($s, $c, $ans) = @_;
    Value->Error('Preview: ' . $ans->{isPreview});
    return 0;
});

BEGIN_PGML
[`\pi=`] [_]{$picmp}
END_PGML
ENDDOCUMENT();
drgrice1 commented 5 months ago

I have known about this problem for a long time. It goes back to how this was for the html2xml endpoint. The issue comes down to the submit button names. For homework assignments and gateway tests the submit button names are "previewAnswers" and "submitAnswers". For the render rpc endpoints the equivalent submit button names are "preview" and "WWsubmit". There is also the "checkAnswers" submit button for homework assignments and gateway test that doesn't have an equivalent for the render rpc endpoints. (You could argue that the "submitAnswers" and "checkAnswers" roles in the prior discussion should be switched relative to the "WWsubmit" button based on the values for the rpc endpoints.)

The issue is that pg assumes the names of the submit buttons are those that homework assignments and gateway tests use. All of the submit buttons listed above are directly reference in PG, and although you bring up the issue for the preview button, the issue exists for the submit answers button as well.

Some possible fixes follow:

One fix is to just change the submit button names in the rpc endpoint templates. This is the simplest fix, but there are systems that use these endpoints that make assumptions on the submit button names. The PreTeXt javascript assumes the submit button name is WWsubmit. I don't think that it uses the preview button name though. Although this approach is the simplest, I don't consider it to be the most versatile solution.

Another fix is to change PG not to use submit button names (accessed via the inputs ref) from the externally created form at all, and instead use translation options that are passed in to PG. This is a more versatile option, but takes more work to implement on the PG side of things.

somiaj commented 5 months ago

I ran across this issue wanting to test feedback in custom answer checkers and make sure preview wasn't giving unwanted feedback (mostly do my checkers honor return 0 if $ans->{isPreview};). In this case another possible fix would be to have pg know that the preview/submit buttons could have different names, and to set things properly if either name was used.

I'm unsure on what the best fix would be, and what sort of considerations would need to be dealt with. It does sound like your last option would be the best and most versatile, but this will require the most work in all the clients using pg and pg itself. So maybe just having pg know both names would cause the least amount of ripples (since either changing the button names or how the pg translator works will cause ripples to fix multiple front ends).

drgrice1 commented 5 months ago

I don't think that adding other submit button names to PG is a good idea. That is counterproductive. We don't want to add another submit button name every time someone decides they want to use a different name.

The easiest solution is to just insist that certain names be used. If we change the preview button for the RPC endpoints as is done in #2311 that already covers all of the important cases. That is the only submit button used for anything of real consequence in pg, and I don't think any front ends care on the preview button name. As to the submit answers button, that doesn't really even matter. Same for the check answers button. The only places that those buttons are used in PG are in the AnswerChecker.pm file for literally nothing, and in the macros compoundProblem.pl, PeriodicRandomization.pl, problemRandomize.pl, and problemPanic.pl which are all either already deprecated or should be.

Alex-Jordan commented 5 months ago

For what it's worth, PreTeXt right now is still using the html2xml endpoint, because I just haven't gotten to updating things there. I would like to make some changes in that area that only apply when PTX interfaces with a 2.19 WW server. We already have to probe the WW server to get its version and do things a bit differently depending on the version. So whatever you might want to change with these names now on develop will not mess things up for PTX. And the sooner changes like that make it into develop, the better for me with updating PTX at the same time.

Now that's just PTX. I can't say what other things (LibreText?) might assume what names for what buttons. I think here at my school there is a chemistry project using H5P spliced with someone mimicking what PTX does, so there's a completely one-off project that might break if I upgrade our WW server to 2.19 and those names change. Not that it couldn't be quickly fixed.

drgrice1 commented 5 months ago

Note that this change is not specific to the html2xml endpoint, but applies to both that and the render_rpc endpoint. Those two endpoints are really the same. The only difference between the two is that the html2xml endpoint converts the 'userID', 'course_password', and 'session_key' parameters to 'user', 'passwd', and 'key' while the render_rpc endpoint does not. The submit button names are the same for both endpoints.

Presumably LibreTexts does not use webwork2 anymore, but uses the standalone renderer which has already changed these submit button names.

somiaj commented 5 months ago

This was fixed with #2311 which has been merged. Can I close this, or is there more discussion to be had?