Skip to content
This repository was archived by the owner on May 15, 2023. It is now read-only.

Conversation

@ackxolotl
Copy link
Member

@ackxolotl ackxolotl commented Feb 28, 2020

This PR adds functionality to send emails to participants by the respective troop managers (might be reused for people in charge of the age sections) and support requests to a support contact.

@ackxolotl ackxolotl force-pushed the mail branch 2 times, most recently from 5ef75e1 to 9d87012 Compare March 2, 2020 13:01
@ackxolotl ackxolotl marked this pull request as ready for review March 2, 2020 13:11
@ackxolotl ackxolotl requested a review from oliverpool March 2, 2020 14:12
@oliverpool
Copy link
Contributor

What is the context of the Pull Request? (which problem does it solve? who requested this feature?)


The support form is a great idea!

We should (later) add some text with more ways to enter in contact :

  • link to this github repo, for the nerds who already have a github account
  • direct email address (if they want to send an email with some attachments for instance)
  • Phone number of someone of the Lagerbüro for emergencies
  • ...

Mass E-Mail:
Wouldn't it be better to offer an export (including the email addresses) and let the StaVo handle it as it pleases?

As-is I see a couple of issues:

  • you don't know for sure who will get your e-mail (how many people don't have an email address in their participant model?)
  • there is no possibility for the participant to reply to the email (adding a Reply-To field would help)
  • What do we do if someone uses this to spam? (without logs, it will be hard to trace)
  • What if our IP gets blacklisted from popular mail servers? (the StaVo will expect all the emails to arrive, but only a fraction will be delivered)
  • How can you send an attachment?
  • some people may receive the email multiple times (my 2 children are participants and both use my email address)

I think that all those issues would be solved, by simply giving an export of the participants:

  • the StaVo can immediately see who has an email address and who doesn't
  • the people can simply hit "reply" to contact the StaVo
  • Spam and blacklist is not our problem anymore
  • He/she can send attachments as he/she wants
  • Deduplication can be easily achieved

@ackxolotl
Copy link
Member Author

What is the context of the Pull Request? (which problem does it solve? who requested this feature?)

I've added a description to the PR.

  • link to this github repo, for the nerds who already have a github account
  • direct email address (if they want to send an email with some attachments for instance)
  • Phone number of someone of the Lagerbüro for emergencies

Good points! Guess I'll add a hint to our GitHub page right away.

Mass E-Mail:
Wouldn't it be better to offer an export (including the email addresses) and let the StaVo handle it as it pleases?

I'd do both, offer data export in various formats and this form for convenience.

  • you don't know for sure who will get your e-mail (how many people don't have an email address in their participant model?)

I can add this information to the form if you want. But the problem whether there is an email address or not does not really depend on whether we have a form to send emails to the participants, right? Even if you export the data, missing email addresses will still be missing. I'd thus strongly encourage people to enter email addresses when registering participants (e.g. for information sent to participants from camp staff).

  • there is no possibility for the participant to reply to the email (adding a Reply-To field would help)

Reply-To is already set since we cannot send emails on behalf of other domains (at least if we don't want them to end up in the spam folder). So the emails are actually sent using an email address of our domain and Reply-To is set to the current user's email address. Should I make this more explicit in the form?

  • What do we do if someone uses this to spam? (without logs, it will be hard to trace)

I've seen this in various other tools, for example our party staff management tool at uni, and there have not been any problems of abuse. I think we can trust people. Besides, every mail server keeps a log of sent emails. So we will know who did what ;)

  • What if our IP gets blacklisted from popular mail servers? (the StaVo will expect all the emails to arrive, but only a fraction will be delivered)

This is a valid point. We should prevent this from happening (setting up everything as compliant as possible, or using a commercial provider like Google G suite or Office 365 Exchange Online as is the case with dpsg1300.de).

  • How can you send an attachment?

People should export the email addresses in this case. However, I'm thinking about providing some formatting features like a Markdown editor.

  • some people may receive the email multiple times (my 2 children are participants and both use my email address)

Yes, and this is okay I think. Besides, I'm also thinking about offering some personalization tags like [first_name] to address participants individually.

I think that all those issues would be solved, by simply giving an export of the participants:

  • the StaVo can immediately see who has an email address and who doesn't
  • the people can simply hit "reply" to contact the StaVo
  • Spam and blacklist is not our problem anymore
  • He/she can send attachments as he/she wants
  • Deduplication can be easily achieved

Well, as I have seen this feature in use before, I do not share most of your concerns. The only real concern I see is preventing the mail server from getting blacklisted. But this problem exists independently of this feature (ok, the server is more likely to get blacklisted if we send out more emails). Nevertheless, since we want to send out emails anyway (e.g. when creating new accounts, to reset passwords, ...) we will have to deal with this issue one way or another. Either by being as compliant as possible or using a commercial product as our district is doing.

@oliverpool
Copy link
Contributor

There are some conflicts (due to the merge of the NaMi lookup).


add a hint to our GitHub page right away.

👍

offer data export in various formats

Ok, lets do it in an other PR

I can add this information to the form if you want.

A simple textual warning should be enough (like "only participant with an email will get your message").

Reply-To is set to the current user's email address.

I think it would be nice to add a form field with this value (prefilled with the user email). It communicates that the people will be able to reply to the mass email and it lets the user change this reply email (if someone else is expected to handle the complaints for instance).

Besides, every mail server keeps a log of sent emails. So we will know who did what ;)

Maybe another textual warning to the user would be good:

  • only use this form to send message strictly related to the organization of the camp (according to the GDPR - DSGVO)
  • we keep a log of every email sent via this form (in case of complaints)

I'm thinking about providing some formatting features like a Markdown editor.

I think we should keep this simple and only support plain text (and encourage the users to use the export for fancier emails).


I find the Recipients field to be a bit confusing as is.

It would probably be better to split it into 2 choices:

  • sections: all sections, beaver, cub, scout, venturer, rover, no section
  • status: all, only leaders, only children

This way it is possible to send an email to:

  • all participants
  • all leaders
  • all children
  • all leaders of a specific section
  • all children of a specific section

And it removes the strange combination currently possible:

  • Recipient=leaders
  • Also send to leaders=false

@ackxolotl
Copy link
Member Author

A simple textual warning should be enough (like "only participant with an email will get your message").

👌

I think it would be nice to add a form field with this value (prefilled with the user email). It communicates that the people will be able to reply to the mass email and it lets the user change this reply email (if someone else is expected to handle the complaints for instance).

Sounds good.

  • only use this form to send message strictly related to the organization of the camp (according to the GDPR - DSGVO)
  • we keep a log of every email sent via this form (in case of complaints)

Ok, that's fine for me.

I think we should keep this simple and only support plain text (and encourage the users to use the export for fancier emails).

I think adding Markdown support wouldn't be that much work using Django MarkdownX (which looks quite fancy). But yes, this is just a shenanigan :)

It would probably be better to split it into 2 choices:

  • sections: all sections, beaver, cub, scout, venturer, rover, no section
  • status: all, only leaders, only children

Ok, I'll do it this way. Thanks for your feedback!

@oliverpool oliverpool mentioned this pull request Mar 29, 2020
13 tasks
@ackxolotl ackxolotl force-pushed the mail branch 4 times, most recently from b406d58 to 460fb03 Compare March 29, 2020 15:54
@ackxolotl ackxolotl requested review from 6543 and oliverpool and removed request for oliverpool March 29, 2020 15:56
Copy link
Contributor

@oliverpool oliverpool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good!

I have some nitpicks, but actually this is very closed to getting merged 👍 !

@ackxolotl
Copy link
Member Author

I've addressed all of your comments. Thanks for your effort!

@ackxolotl ackxolotl requested a review from oliverpool March 29, 2020 18:04
Copy link
Contributor

@oliverpool oliverpool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor cleanups and we are good to go!

@ackxolotl ackxolotl force-pushed the mail branch 2 times, most recently from 5e24f7e to 23aab0e Compare March 29, 2020 18:51
Copy link
Contributor

@oliverpool oliverpool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@ackxolotl ackxolotl merged commit b5faf13 into master Mar 29, 2020
@ackxolotl ackxolotl deleted the mail branch March 29, 2020 19:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants