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

Send notification content along with notification email #4428

Open
andreagrandi opened this Issue Oct 17, 2018 · 6 comments

Comments

Projects
None yet
5 participants
@andreagrandi
Member

andreagrandi commented Oct 17, 2018

Problem

As a mentor and as a student too, when I get a notification by email, it's usually something like this:

Hi there,

A student you are mentoring (***************) has posted a new comment on their solution to Kindergarten Garden on the Python track. You can view it here.

The email doesn't have much information. It could be a "oh thanks!" or "no way, I'm not doing that". We don't know until we go to the website and click through the links.

Possible solution

Send the content of the comment along with the notification email. In theory we could also email an html formatted version of the updated code (I'm pretty sure there are libs which can do this, no need to reinvent the wheel) and we could "Approve" just clicking in a link in the email.

@sshine

This comment has been minimized.

sshine commented Oct 17, 2018

Since I looked at how one may do this earlier, the implementer may find this a head start:

Looking at the codebase, it seems that there are two templates 1 (plaintext, haml) that need modification. The mailer is in 2, and the model in scope that appears to contain the comment is DiscussionPost 3. There is an example of Markdown-to-HTML in 4; I don't know if there's another function for embedding this in HAML.

@tehsphinx

This comment has been minimized.

tehsphinx commented Oct 17, 2018

My two cents: It might be good to limit the amount of emails a mentor receives on a solution to 1 (one eMail with comment) until the mentor looks at that exercise again. 2 reasons:

  • mailboxes do not get spammed with a few mentored exercises
  • it is a lot easier to avoid an "angry students" comments. A mentor only gets the first mail and not all 40.
@andreagrandi

This comment has been minimized.

Member

andreagrandi commented Oct 17, 2018

@tehsphinx it's a very good point. I only think it could be a separate task/issue rather than a single one. In this case the point is more about the content rather than on the number of notifications. Let's wait to get some comments from maintainers who may have know if we can easily combine the two things or if they should be a separate task.

@iHiD

This comment has been minimized.

Contributor

iHiD commented Oct 17, 2018

I think that's a valid suggestion @tehsphinx. Could you open a seperate issue for it pls?

So I think I'm pro this idea now. It was very popular on the Slack, and if it makes mentors' lives easier than that's important. The original reason not to do it was to avoid possible abuse scenarios but that's not been a huge problem for the last 3 months, so I'm ok with trying this.

@kytrinyx @nicolechalmers I would appreciate any thoughts you have.

@kytrinyx

This comment has been minimized.

Member

kytrinyx commented Nov 13, 2018

I think it would be worth trying it. We can always change our minds (and it sounds like it wouldn't be massively difficult to do).

@iHiD

This comment has been minimized.

Contributor

iHiD commented Nov 14, 2018

Added to the roadmap with a medium-ish sort of priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment