ficlatte / main

Ficlatté website main code
GNU Affero General Public License v3.0
2 stars 2 forks source link

Email notification is empty when comment body is empty. #17

Closed HSAR closed 7 years ago

HSAR commented 7 years ago

If a user leaves a rating without a comment, the email notification(s) that are sent out are a bit nonsensical.

stitzelj commented 7 years ago

Interesting. I was just looking at the mail templates yesterday. I'll take another peek and see what's happening with the logic on that one. Do you have an example to show?

HSAR commented 7 years ago

Can do, here's the text:

Hi.
This is the Ficlatte server.  You are currently subscribed to receive notifications of new comments posted to Ficlatte story "Yes".

Fantasist just posted a comment:

To see the comment at Ficlatte, click here:
https://ficlatte.com/stories/40957/
To stop receiving notifications of comments on this story, click here:
https://ficlatte.com/stories/unsubscribe/40957/
To adjust your e-mail preferences, update your profile here:
https://ficlatte.com/authors/u/profile/

Keep writing!

The Ficlatte team
stitzelj commented 7 years ago

Ok, that makes sense. The logic should be easy enough to update to only send a notification when there's comment text present.

HSAR commented 7 years ago

I agree.

HSAR commented 7 years ago

Warning: I have pending changes to the email sending file, so get yours in first so you don't have to be the one that deals with the merge conflict ;)

stitzelj commented 7 years ago

I think you're fine to commit. My changes don't actually use the mail.py file, so there shouldn't be any conflicts there.

HSAR commented 7 years ago

Oh, my work is untested, and my conscience won't permit me to issue a pull request until I have some automated tests, so I'm blocked by my slowness to finish https://github.com/ficlatte/main/issues/8.

stitzelj commented 7 years ago

Well, in that case... :)

stitzelj commented 7 years ago

Fixed the logic for this issue. Email notifications will now only go out if the comment submission actually includes text.

stitzelj commented 7 years ago

This is resolved with #15, so this issue can be closed.

/cc @ethel-t-frog

ethel-t-frog commented 7 years ago

Fixed by Jim