Skip to content

Conversation

@drdrew42
Copy link
Member

@drdrew42 drdrew42 commented Aug 17, 2019

Purpose

Our institution uses the feedbackMacro_form function to direct students to ask questions on a Q&A site (https://openlab.citytech.cuny.edu/ol-webwork) where their questions are aggregated based on having the "same" problem (same pg file, different seeds). That way students have an FAQ on a per-problem basis.

We need some additional info from WeBWorK because the Q&A site does not share accounts with WeBWorK.

Description

Okay, so - as suggested ( #957 ) - I restructured my expansion of (the necessary, for our Q&A WordPress plugin) form data sent through the feedbackMacro_form function.

  1. I removed my changes from ContentGenerator.pm and instead expanded the parameters passed to feedbackMacro when it is called from output_email_instructor in CG\Problem.pm and output_footer in CG\ProblemUtil\ProblemUtil.pm

  2. In the process, I found multiple clusters of repeated code - so I shifted them into their own subroutines in WeBWorK::Util. The repositioned code now comprises two new subroutines: fetchEmailRecipients and generateURLs (which are made available through ContentGenerator.pm). These new functions helped to simplify the process of adding to the form data sent through the feedbackMacro

  3. Please forgive the somewhat-large diffs on ContentGenerator.pm and Problem.pm - I have been using Atom as my editor, and it auto-strips trailing whitespace. The only relevant change to ContentGenerator.pm is line 62 (loading the new functions from Util); and in Problem.pm, the only meaningful change is to output_email_instructor starting on line 2116.

Testing

  1. One should start by testing that their usual email functions are not affected by these changes.
  2. As a student, use the email instructor button and confirm that it works as expected
  3. Load up a JITAR set (make sure that there are a limited number of attempts!) - and, as a student, trigger the jitar-warning email by exhausting your allotted attempts. Confirm that an email is sent to the instructor (test TA account too?) warning that a student failed to complete a JITAR set.
  4. add the following to the course.conf:
    $courseURLs{feedbackFormURL} = 'http://localhost/test.php';
    $feedback_button_name = "Test the PR"
  5. create /var/www/html/test.php with the contents:
<html>
<head><title>PHP Get Results</title></head>

<body>

<?php

// Show all form data submitted
foreach($_POST as $key=>$value){
    echo $key, ' => ', htmlspecialchars($value), "<br/>";
}

?>

</body>
</html>
  1. Visit a problem page and click the (used to be email instructor) button that says "Test the PR"; confirm that a new page opens with a bunch of key => values

New data provided

  • emailableURL => http://localhost/webwork2/CourseID/problemSetName/problemNumber/?showOldAnswers=1&effectiveUser=student&displayMode=MathJax
    (this was the only thing instructors missed about the email instructor button - we bring it along so that the Q&A site can include it with the notification email)

  • randomSeed => 2013
    (not used by our Q&A site yet, but may be useful in re-rendering the student's problem -- XMLRPC -- instead of decoding the pg64 string that gets sent)

  • notifyAddresses => Administrator <fake@email.com>;"Prof. Lastname" <lastname@uni.edu>
    (all users who have permission to receive feedback emails - now our Q&A site is informed of who should receive notification that this user asked a question...)

  • problemPath => local/folder/some-problem.pg
    (this is our unique ID for problems. It maintains a consistent basis for identifying "identical" problems when instructors can remove or reorder problems...)

  • studentName => active user's Full Name
    (because our Q&A site does not share single sign-on with WeBWorK - so we can send notifications to instructors/TAs and they 100% know who the student is)

@taniwallach
Copy link
Member

Since this is a PR for develop and not planned for inclusion in WW 2.15, I think you may need to:

  1. wait for WW 2.15 to release before it gets attention, and
  2. request reviews (after WW 2.15 releases) to bring it back to attention.

@mgage
Copy link
Member

mgage commented Dec 25, 2019

This PR has conflicts with develop. I need guidance as to which changes to accept in the Problem.pm file

@mgage mgage self-requested a review December 25, 2019 17:21
@mgage
Copy link
Member

mgage commented Dec 28, 2019

@drdrew42 -- Andrew could you take a quick look and revise this pull request so that it doesn't have conflicts with develop? Thanks.

@drdrew42
Copy link
Member Author

Thankfully, that can be done without a development environment ;)

@mgage
Copy link
Member

mgage commented Dec 29, 2019

Checked standard reporting of feedback by email.
standard emailing all students works (it shouldn't have been affected and was not)
reporting JITAR failures works fine
adding the reconfiguration and posting to test.php works also

Looks good.

@mgage mgage merged commit abf5a80 into openwebwork:develop Dec 29, 2019
@drdrew42 drdrew42 deleted the feedback_form_revision branch December 29, 2019 20:19
mgage added a commit to mgage/webwork2 that referenced this pull request Jan 29, 2021
The setName and problemNumber were unavailable when called from the Feedback page because the calling path contained in $r has changed
Perhaps there is a simpler fix than what I have done

The bug appeared around August 17, 2019 SHA b231339   "moved email retrieval to WW::Utils

and was merged into develop branches around December 29,2019 with PR openwebwork#986   "refactoring...."  SHA abf5a80

This was a tough bug to track down
@Alex-Jordan
Copy link
Contributor

I'm essentially using the develop branch now in production, so it's basically a field test of a lot of changes for 2.16.

I think there is an issue with this PR. The Email Instructor messages are no longer returning a link to the precise location from which a student sent the message. For example, if I go into a problem set, go to problem #2, click Email Instructor, and send a test message, I receive a message with this in it:

 Click this link to see the page from which the user sent feedback:
https://webwork.pcc.edu/webwork2/courseName/?effectiveUser=alex.jordan

But the link should at least include the problem set and problem number. Also these links have included selectors like this in the past:

?showCorrectAnswers=&showOldAnswers=1&displayMode=MathJax&showSolutions=&showHints=1

I think this PR is the source of the change in behavior because the generateURLs subroutine is what Feedback.pm uses to build the link in the feedback message, and this PR adds that subroutine. I haven't looked at it closely, but @drdrew42 does this sound like it could be the source?

@somiaj
Copy link
Contributor

somiaj commented Feb 1, 2021

Seems I too see the email issue, and links now don't go to the problem the email was sent form, as it isn't included in the url anymore. Thanks for sharing this info.

drgrice1 added a commit to drgrice1/webwork2 that referenced this pull request Feb 1, 2021
feedback page.

This was introduced in pull request openwebwork#986.

That pull request switched from obtaining the set and problem ids cached
in the WeBWorK::ContentGenerator::Problem object to attempting to find
the set and problem ids from the url parameters.  The problem is that
the feedback page does not have those url parameters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants