change introduction to feedback emails

[Alex-Jordan]

This changes some things about the opening lines to a feedback email.

This may be controversial. I'm happy to make changes. This started out simply as removing the double space that was there, but then I kept going. I think it's nicer this way, but not opposed to further changes or retractions.

[Alex-Jordan]

Here is what an email looks like with this:

This message was generated by WeBWorK at
https://webwork-hosting.runestone.academy/webwork2
in response to a request from Alex Jordan (alex.jordan) at
162.158.63.39:20448

To visit the page from which the user sent feedback, go to:
https://webwork-hosting.runestone.academy/webwork2/Active-Calculus-Demo/1_1-How_do_we_measure_velocity_Exercises/1?effectiveUser=alex.jordan&showSolutions=1&showCorrectAnswers=1&displayMode=MathJax&showHints=1&showOldAnswers=1

Alex Jordan (alex.jordan) wrote:


I need help with this exercise!

Maybe the ***** The feedback message: ***** part should be retained. I don't know.

What is supposed to be the point of the IP address? When I initiated this message, I was at home using PCC's VPN, logged in to a course on the Runestone WW server. I looked up this IP address and it's somewhere in New Jersey. I turned off the VPN and then I get the same IP address, so it's not some mysterious thing about PCC's VPN. Also, it is not the IP address for the Runestone WW server.

[somiaj]

I would be okay with simplifying the whole first paragraph. If someone prefers to include the users ip, maybe an option to not include it would be nice. I was thinking along the lines of of Message from <user> using WeBWorK at <course url>.

[drgrice1]

This looks fine to me.

[drgrice1]

I like the addition of the user name and login id to the email message. I would be fine with removing the remote host and port from the email. I doubt that is actually useful to anyone in reality. Remote host ip addresses rarely are anymore.

[drgrice1]

Another idea would be to move the host ip and port down with the data about the user at the end of the email.

[somiaj]

Here are some more ideas, modify what you think is best, just thinking of other things that could be updated.

If wanting to keep the ip address, maybe add that to the $mail{feedbackVerbosity}='debug' blocks, but omit it in other cases.

Also since you are editing this and the user's id and full name is now included by default, you might want to update the output for both standard and debug output. The format_user data also gives the user id, full name, and email (since it is the from: field), so consider if duplicated output is useful (or maybe only include the user id or full name in your changes as the user data will include the full set of info). You may also want to update the other headers like $msg .= "***** Data about the user: *****\n\n"; to match the changes you have already made as well.

[Alex-Jordan]

I'm making a few changes to this now as suggested here.

One more change I would like to make is have it so that when the email comes from a problem, the URL would include showProblemGrader=1. But I can't work out how to do that. Simply adding showProblemGrader to line 355 of lib/WeBWorK/Utils.pm is not enough.

[Alex-Jordan]

Now an email looks like:

Message from Alex Jordan (alex.jordan) via WeBWorK at
https://webwork-hosting.runestone.academy/webwork2

To visit the page from which the user sent feedback, go to:
https://webwork-hosting.runestone.academy/webwork2/Active-Calculus-Demo/1_1-How_do_we_measure_velocity_Exercises/1?effectiveUser=alex.jordan&showSolutions=1&showCorrectAnswers=1&displayMode=MathJax&showHints=1&showOldAnswers=1

Alex Jordan (alex.jordan) wrote:


I need help with this exercise!


***** Other stuff, if verbosity is high *****

[drgrice1]

I just noticed something wrong with the feedback page. It is not caused by this pull request, and is also in develop. The "Cancel E-mail" link shown before email is sent, and the "Return to your work" link shown after sending an email also get the parameters from the generateURLs method. That means that the parameters showCorrectAnswers, showHints, showSolutions, etc., are all added to those links. Those shouldn't be there. I noticed this because I tested sending an email as my instructor user, and when returning to the page the correct answers were all shown. Those links really shouldn't come from that method at all.

Sorry, this is a sidetrack from what this pull request is about, but I wanted it noted.

[drgrice1]

One more change I would like to make is have it so that when the email comes from a problem, the URL would include showProblemGrader=1. But I can't work out how to do that. Simply adding showProblemGrader to line 355 of lib/WeBWorK/Utils.pm is not enough.

I think that you are misinterpreting what it means to pass the params option to systemLink as an array reference as is done in generateURLs. It doesn't mean add those parameters to the link. It means add the values of those parameters from the current request to the link if they are set. So the current things in the @args array are added to the link if those parameters were set for the user's request that is sending the email.

Note that many of the parameters are passed from the problem to the email form when the feedbackMacro method is called in lib/WeBWorK/ContentGenerator/Problem.pm and the feedback_macro_email.html.ep template is rendered. But that is still only in the case that those parameters were set for the user on the problem page to begin with.

What you could do is change what is passed to the systemLink call to a hash reference instead of an array reference. Anything in the hash that has a value of undef will work in the same way as for the array approach. However, if you add showProblemGrader => 1 instead of showProblemGrader => undef to the hash, then it will say to use the value you give rather than use the value from the current request parameters.

If you do that (and even if you don't) the params argument should not be passed to the systemLink call used for the $returnURL. That is the reason for the issue that noted in my other comment. So line 368 in the generateURLs method of Utils.pm should be changed from $returnURL = $c->systemLink($routePath, params => [@args]); to $returnURL = $c->systemLink($routePath);.

[drgrice1]

By the way, you should note that there is actually never any reason that any of those parameters should be in the $returnURL since that is only ever used for internal links back to the page the user came from before clicking the "Email Instructor" link.