OneZoom / OZtree

OneZoom Tree of Life Explorer
Other
90 stars 20 forks source link

Renewal e-mails to sponsors #439

Open lentinj opened 2 years ago

lentinj commented 2 years ago

Collating renewal e-mail tasks from various places.

We need to send e-mails to users when:

Also:

lentinj commented 2 years ago

From #362:

This script should check if a species to be renewed is no longer in the tree - if it is no longer in the tree then we extend the sponsorship duration for 90 days in the hope that it reappears and knowing that no-one else can sponsor it nor should it be charged for renewal.

@jrosindell Are OTTs flitting in and out of existence a likely scenario? I don't think renewing it for free is the right approach due to the database spew it will generate, adding new renewal rows every 90 days if an OTT permanently disappears.

Obviously asking someone to renew a non-existent OTT isn't good, but if it only stopped existing for a week, then they'd still get an e-mail after it reappeared. Sure the OTT may have expired by that point, but it's not like anyone could have stolen it for themselves in the interim.

@jrosindell reminder email questions:

I'll make up some templates and link to them here, so it's easy to edit afterwards.

lentinj commented 2 years ago

@jrosindell if we don't e-mail when sponsorship_text_level is high, do we e-mail a onezoom admin mailbox instead, so you know to write one? If so, what should that be?

jrosindell commented 2 years ago

Giving some answers to the above - it's rare that sponsored OTTs flick in and out of existence. We've had I think 3 or 4 cases of it since we launched (out of 800 odd sponsored leaves in total) - and these are mostly not flicking out and coming back - they are flicking out and that's it. Ideally such cases would be automatically handled and I was thinking of an easy way to do this. Maybe these leaves should simply not be moved out of the original reservations table - they are not 'renewed' in the real sense the end date is just extended. This is I think what I intended when I made this original comment.

jrosindell commented 2 years ago

Responding in line to the next question I think soon is 90 days but really soon is maybe 21 days as 2 week holidays from e-mail are not uncommon.

yes I think one e-mail with a list of all those coming up makes sense. That makes it actually a bit complex because a list might have variable renewal dates. Perhaps to resolve this we disaggregate the trigger conditions from the e-mail text. So, the text will list in order everything coming up over the next 4 months say and then we need to decide how frequently to send these messages and how to note this in the two given database fields - this will require discussion with @hyanwong

Under this model it's OK to combine urgent renewals of some species with first reminders of others in the same message.

jrosindell commented 2 years ago

I think

When they trigger an e-mail manually via. sponsor_renew_request (E-mail sender from sponsorship #430)

is a nice touch here.

jrosindell commented 2 years ago

Another factor is what if they don't want to receive even these reminders despite having sponsored a lot of species in the past (either because they've already diarised the dates or don't want to renew). This is an additional case we haven't considered before.

jrosindell commented 2 years ago

if we don't e-mail when sponsorship_text_level is high, do we e-mail a onezoom admin mailbox instead, so you know to write one? If so, what should that be?

I think yes we should do that well in advance of the date so that the OneZoom administration have time to write to the diner and hand edit the database with the reminder sent fields.

Probably it should failsafe and send a reminder anyway in case the admin take no action to record the date message sent.

These admin e-mails should include the same model of a list of species and expiry dates in order so that it's easy to copy paste into a new message.

lentinj commented 2 years ago

they are flicking out and that's it. Ideally such cases would be automatically handled and I was thinking of an easy way to do this. ... Maybe these leaves should simply not be moved out of the original reservations table.

Then I'm not getting the advantage of doing this vs just letting them expire as you would any other row. The only place their sponsorship will be visible with a dead OTT is the total counts, which will now include sums from expired_reservations anyway, as will the donor_list (eventually).

However, the renewals page should explain the situation properly, that the OTT they sponsored is no longer available - I've made #440.

yes I think one e-mail with a list of all those coming up makes sense. That makes it actually a bit complex because a list might have variable renewal dates. Perhaps to resolve this we disaggregate the trigger conditions from the e-mail text.

Exactly. I was thinking 2 lists: "These species are coming up for renewal", "This is your final reminder to renew these species".

Another factor is what if they don't want to receive even these reminders

Yes, an "unsubscribe" link seems wise. Do these get sent regardless of "Allow further contact?" (I've been assuming yes). If so the link could set the times for all emailed_re_renewal_initial to be 1970, we won't e-mail again since the date has been filled in, and it's pretty obvious it didn't actually happen. Obviously future sponsorships won't get unsubscribed, but there's nowhere to hang that information as yet.

jrosindell commented 2 years ago

Quick update - Yan and I have decided not to use the sponsorship text level field in any way - so just leave it and treat everyone the same.

Separately from this we may send bespoke e-mail to high value donors personally and fill out the DB fields by hand so that they are not auto mailed because they system will know it's already done.

jrosindell commented 2 years ago

Yes, an "unsubscribe" link seems wise. Do these get sent regardless of "Allow further contact?" (I've been assuming yes). If so the link could set the times for all emailed_re_renewal_initial to be 1970, we won't e-mail again since the date has been filled in, and it's pretty obvious it didn't actually happen. Obviously future sponsorships won't get unsubscribed, but there's nowhere to hang that information as yet.

We have a field 'restrict_all_contact' already (which is distinct from 'allow_contact') and if this is TRUE then no e-mail at all should be sent out. If this is false then reminders should be sent regardless of the 'allow_contact'

An unsubscribe link just needs to flip 'restrict_all_contact' to true for all rows with that username

jrosindell commented 2 years ago

Responding in line to the next question I think soon is 90 days but really soon is maybe 21 days as 2 week holidays from e-mail are not uncommon.

yes I think one e-mail with a list of all those coming up makes sense. That makes it actually a bit complex because a list might have variable renewal dates. Perhaps to resolve this we disaggregate the trigger conditions from the e-mail text. So, the text will list in order everything coming up over the next 4 months say and then we need to decide how frequently to send these messages and how to note this in the two given database fields - this will require discussion with @hyanwong

Under this model it's OK to combine urgent renewals of some species with first reminders of others in the same message.

After discussion we're suggesting that all e-mails sent have an ordered list of sponsorships coming up for renewal (anything within 90 days).

In order to avoid repeated e-mails for sponsorships with similar renewal dates we propose the initial and final reminders have both a trigger day and a max day.

First reminder trigger = 75 days First reminder max = 90 days Final reminder trigger = 15 days Final reminder max = 30 days

The cron job for sending reminders checks against the trigger but anything up to the max can be counted as having been sent early (and hence the emailed_re_renewal_initial or emailed_re_renewal_final fields can be set and when the trigger day comes it won't trigger a further message.

jrosindell commented 2 years ago

if we don't e-mail when sponsorship_text_level is high, do we e-mail a onezoom admin mailbox instead, so you know to write one? If so, what should that be?

I think yes we should do that well in advance of the date so that the OneZoom administration have time to write to the diner and hand edit the database with the reminder sent fields.

Probably it should failsafe and send a reminder anyway in case the admin take no action to record the date message sent.

These admin e-mails should include the same model of a list of species and expiry dates in order so that it's easy to copy paste into a new message.

We're now not needing this functionality and I will instead open a new issue (which @hyanwong has agreed to work on) in for form of a management page enabling us to get the necessary list and links as plain text for any username or e-mail.

lentinj commented 2 years ago

Right, the above should send the full reminder e-mail when you manually trigger it. The content of these e-mails should basically be the same as the automatic reminder e-mails, we just don't trigger any of the first/final reminder shenanigans. Again, if you don't have a mail setup in your instance then the contents will be displayed on-screen.

I've included all active sponsorships in the output, not just the expiring ones. The reason for this is I may go back and use the same output to generate the renewals page itself (which is backed by a very similar query). I also thought it would be nice to say thanks when asking for renewals.

E-mail template is here, plain-text only currently: https://github.com/OneZoom/OZtree/blob/3.6/views/sponsor_renew_reminder.txt

The guts of the logic of when we'll send automatic e-mails and what goes in what list is here, which may prove interesting:-

https://github.com/OneZoom/OZtree/blob/39021c814e05d2011b8fc2323adf5c92c58c8bfe/modules/sponsorship.py#L691-L707

With the above we also get unsubscribe links to toggle restrict_all_contact, however we always send e-mails if they type their e-mail address in the box.

hyanwong commented 2 years ago

Ad we do in manage.py, could we also check that as well as having a mailer set up, smtp.autosend_email is true (or 1) before sending the emails from the sponsorship.py page, otherwise we display to screen? That way we can turn of email testing (e.g. on beta) even though beta has the mailer info set up.

hyanwong commented 2 years ago

Instead of compiling a list per email address (when for_email is defined), I think we should probably be doing it per username, and picking the most recent sponsorship which has a reservations.e_mail defined (or the reservations.PP_e_mail address if nothing else can be found). We have a number of donors who are likely to have multiple emails, or who may have forgotten to set up an email for certain sponsorships. The (new) username field should be the canonical way to get all stuff for a given donor.

lentinj commented 2 years ago

I think we should probably be doing it per username, and picking the most recent sponsorship which has a reservations.e_mail defined

In that case what stops me from buying a leaf with my e-mail address but your name (which will then get assigned to your username I think?), and then all future e-mails about your leaves will go to me?

We have a number of donors who are likely to have multiple emails, or who may have forgotten to set up an email for certain sponsorships.

Yes, given the form doesn't stress e-mail very much that does seem likely. We could fall back to PP_e_mail, but would it be easier to populate e_mail from PP_e_mail if not already defined?

(EDIT: Note that manage.py already does e_mail or PP_e_mail)

lentinj commented 2 years ago

Right, this is getting there. We need to now hook sponsorship_email_reminders into something that can be periodically called. This could be added to background_tasks, but I'm getting more tempted to make it it's own command-line tool, so:

We also need to resolve the debate about username above. At very least fallback to PP_e_mail should be looked into.

lentinj commented 2 years ago

We also have the matter of HTML vs. plain text e-mails. I'm currently making life simple with plain-text only, but will result the links (with full signature) being visible in the e-mails.

We can have an HTML and plain text template easily enough, but then both will need to be kept in sync.

lentinj commented 2 years ago

The above adds a Grunt task to send the e-mails, see the top of the file for usage.

Annoyingly you have to supply the public hostname of the instance on the command line, since web2py has no idea what that might be, as the request hasn't gone through Nginx. The only other option would be a URL that you curl to trigger the job, but the downsides of that would be accidentally exposing it to the greater world, or Nginx timing out and leaving the job in limbo.

2 questions above still stand too.

hyanwong commented 2 years ago

I think we should probably be doing it per username, and picking the most recent sponsorship which has a reservations.e_mail defined

In that case what stops me from buying a leaf with my e-mail address but your name (which will then get assigned to your username I think?), and then all future e-mails about your leaves will go to me?

I think (hope!) that we allocate the username based on sharing the email address. So buying under the donor name Yan Wong won't get allocated to the username YanWong unless the emails match. But we should definitely verify (?test) that this is the case.

Yes, given the form doesn't stress e-mail very much that does seem likely. We could fall back to PP_e_mail, but would it be easier to populate e_mail from PP_e_mail if not already defined?

People might have old emails in the system. I think we should take the most recent e_mail for a username, and if there are no e_mails defines, take the most recent PP_e_mail. This is rather convoluted logic, so I think we should do this every time, not have a one-off script to population e_mails. We should probably have a function in the usernames.py module to get the most recent email for a username like this?

jrosindell commented 2 years ago

We should remember that users might enter any e-mail onto the page that will send them a reminder to that e-mail address even if it's not the most recent one for that user

Another annoying edge case - is it possible that the same e-mail ends up getting associated with more than one different username? E.g. because on set of sponsorships have the same PP e-mail but different user entered e-mails - then one of those user -emails appears against a different PP e-mail as well.

What should the procedure be for if a user wants to consolidate their e-mail addresses - which ones do we edit? Do we edit the PP e-mails as the primary ones even though we then lose the information of what the original PP e-mail was

hyanwong commented 2 years ago

We should remember that users might enter any e-mail onto the page that will send them a reminder to that e-mail address even if it's not the most recent one for that user

Yes, that's OK though, I think? There is the edge case you just brought up below though, which does need thinking about.

Another annoying edge case - is it possible that the same e-mail ends up getting associated with more than one different username? E.g. because on set of sponsorships have the same PP e-mail but different user entered e-mails - then one of those user -emails appears against a different PP e-mail as well.

Yes, it's perfectly possible that the same email address is associated with different usernames. Normally that would be fine: the problem will be when someone types in that email address to be sent an update on their sponsorships. I think in this case we assume that the email is valid for both usernames, and (perhaps) send 2 emails, one for each username. It's so rare that I think extra emails are a reasonable solution to this.

What should the procedure be for if a user wants to consolidate their e-mail addresses - which ones do we edit? Do we edit the PP e-mails as the primary ones even though we then lose the information of what the original PP e-mail was

No, we never consolidate. We always take the most recent one. If someone wants to change their email, we either (a) get them to sponsor under the new email, in which case the new email is always the one we use for that username or (b) hand edit their e_mail (not PP_e_mail) field in the DB for their most recent sponsorship, which will always be the one used for that username.

The only time we would use an older email address is when a user types their old e-mail into the "email me my details" form, which would still be OK (I think). Something to decide here though: do we send the details to the email address they entered, or to the most recent one for that username? If we send it to the one they entered, it means that previous email addresses (which in the worst case, could have been hacked) give access to their account. If we send it to the new one, then if they type in the wrong email in their most recent sponsorship, there is no way for them to reset it. My preference would probably be for the latter.

Come to think of it, the "send me my details" page should allow a username to be typed, as well as an email, I reckon (easy to do: if it has an "@" it's an email). It should also not acknowledge whether or not an email or username matched one in our database, but just say "if that username or email matched one in our records, you will have been sent your sponsorship details to the most recent email address we have on record for you. If you do not receive an email, check your junk mail folder, and if there is still nothing there, please contact us directly."

lentinj commented 2 years ago

There's several cases here to think about:

  1. Collation of sponsorship entries inside sponsorship_email_reminders, i.e. what do we group the reservation rows by before sending?
  2. Resolving that group of reservation rows to an e-mail address.
  3. How do we go from an e-mail address typed into the "remind me" box to one of these collations?

Currently it's over simple, the answer to everything is the e_mail field.

I think (hope!) that we allocate the username based on sharing the email address.

In which case for collation is using the username field going to offer anything further than using e-mail? I guess we can rely on it having resolved the e_mail vs PP_e_mail differences for us, which is probably the important bit.

For (2) and (3) we need a username_to_email() and get_username_for_email(). The former isn't that much of a deal I don't think, we pull an e-mail address out of the most recent entry. The latter though is definitely a can of worms.

I think in this case we assume that the email is valid for both usernames, and (perhaps) send 2 emails, one for each username. It's so rare that I think extra emails are a reasonable solution to this.

Okay, that makes it less wormy, but still all a bit odd.

Something to decide here though: do we send the details to the email address they entered, or to the most recent one for that username?

I don't think either option is necessarily better security-wise, but the latter involves a lot less special cases which to me suggests it's the right option.

jrosindell commented 2 years ago

Something to decide here though: do we send the details to the email address they entered, or to the most recent one for that username? I don't think either option is necessarily better security-wise, but the latter involves a lot less special cases which to me suggests it's the right option.

Agreed

Currently it's over simple, the answer to everything is the e_mail field.

This does make sense to me and keeps it very simple. People will just have to accept that if they use different e-mails all over the place this leads to issues and they will have to make some effort to consolidate them. So I think in summary it should be using e-mail and not username for sending out the messages - from that it follows that perhaps the e-mail provides a link to all usernames with a connection to that e-mail

No, we never consolidate. We always take the most recent one. If someone wants to change their email, we either (a) get them to sponsor under the new email, in which case the new email is always the one we use for that username or (b) hand edit their e_mail (not PP_e_mail) field in the DB for their most recent sponsorship, which will always be the one used for that username.

OK so this suggests their entered e-mail has preference over the PP email - good - that resolves one of my concerns

hyanwong commented 2 years ago

Currently it's over simple, the answer to everything is the e_mail field.

This does make sense to me and keeps it very simple. People will just have to accept that if they use different e-mails all over the place this leads to issues and they will have to make some effort to consolidate them. So I think in summary it should be using e-mail and not username for sending out the messages

I'm not sure I understand here. The main point of creating a username field was to consolidate all sponsorships into a uniquely identified person. I don't think we can consolidate emails because we want to keep (at least) the PP_email field intact and unchanging.

  • from that it follows that perhaps the e-mail provides a link to all usernames with a connection to that e-mail

Hmm, not sure what you mean here?

OK so this suggests their entered e-mail has preference over the PP email - good - that resolves one of my concerns

Yes

hyanwong commented 2 years ago

In which case for collation is using the username field going to offer anything further than using e-mail? I guess we can rely on it having resolved the e_mail vs PP_e_mail differences for us, which is probably the important bit.

Yes, because lots of people do have multiple email addresses, or even no e_mail in the DB. We can use the username field to identify unique donors and adjust it by hand in the DB if necessary (I have done this in some cases)

hyanwong commented 2 years ago

There's several cases here to think about:

  1. Collation of sponsorship entries inside sponsorship_email_reminders, i.e. what do we group the reservation rows by before sending?

I would say that we always group by username, and send one email out per username. The donor list will be per-username too. The only time we refer to e_mail is when the user themselves types it in to the "send me my details by email" box (and they could also type their username in there)

  1. Resolving that group of reservation rows to an e-mail address.

We take all the rows for a given username and return the most recent e_mail, or most recent PP_e_mail if all e_mails are NULL.

  1. How do we go from an e-mail address typed into the "remind me" box to one of these collations?

We look for all usernames associated with that email, and create an array of usernames. Then we loop over the array and find all the sponsorships for each username in turn, and send an email out using the email address as resolved in (2) above. If there is more than one username in the array, then we will send out multiple emails, but 🤷

Something to decide here though: do we send the details to the email address they entered, or to the most recent one for that username?

I don't think either option is necessarily better security-wise, but the latter involves a lot less special cases which to me suggests it's the right option.

Agreed.

lentinj commented 2 years ago

Okay, the above mega-commit should switch everything I could think of over to keying on username. You may want to look closer at modules/usernames.py and decide if I'm doing the right thing in each case.

lentinj commented 2 years ago

Remaining TODOs:

lentinj commented 2 years ago

The above hides the "unknown e-mail address" error, but only when is_testing = False. This way we don't confuse ourselves excessively.

lentinj commented 2 years ago

There's plenty enough here without going into the thankyou e-mails, which are a separate kettle of fish. Opened #452 for that.

lentinj commented 1 year ago

I'm getting cold feet about having separate jobs, one for expiring sponsorships, one for renewal reminder e-mails. The rationale for 2 separate jobs was that the expire job could run a lot more frequently than the e-mailing job. But do we really care if we only expire sponsorships whilst sending e-mails?

We don't seem to have decided how often we run the renewal-reminder cron job either. It doesn't really matter in an ideal world, since we only send e-mails once. But it's a useful rate-limit to make sure we don't bombard users accidentally.

What about a single job, that:

...and we can run this either manually, with a dry-run option that applies to everything, or default to running it once an hour?

lentinj commented 1 year ago

Another thing that we seem to have missed, we don't remind people for expired sponsorships. We can get around this by sending reminder e-mails, then moving expired entries. Which is another reason to combine the 2 jobs, otherwise we have a race condition between the two.

But I wonder if the more sensible thing is to have a separate section for "these are the things that you have sponsored in the past". If one of these is missing the final_reminder, then we send the e-mail.

lentinj commented 1 year ago

The trigger logic from above also seems to have fallen through the cracks. I think the rationale was that if we said "only run every week", we didn't need to bother, since unless they made one reservation a week they'd get all their reservations grouped.

But if we run the reminder emails at a more frequent schedule, this needs to come back.

I'm still thinking this will make it easier to integrate into the setup and logging long-term, and have one supervisord job that does "background_tasks && wait", which will then log into the same places & alert as the main job.