Send license expiration notification emails

[xmedr]

Overview

This creates a management command send_expiration_email that puts details for all licenses expiring within a year of the command's start date into an email, which will be sent to users in the NotificationSubscription model. The intention is that this will be run once a year with a cron job.

There is a fix here for an error with validators that would not allow for migrations to be made. The fix adds @wraps decorators to certain validator functions.

This also adds an .env.example file intended to store the mandrill credentials and adds more context to local_settings.example.py for email sending.

Closes #106
Connects #63

Notes

For testing, my local db didn't have any licenses that were expiring this year, so I brought the app up and made one.

Also, this does not include work done to make a cron job. To my understanding, it seems like that's done on the deployment instance.

Testing Instructions

• Copy the contents of .env.example to a new .env file and plug in the mandrill api key
• Run migrations for the new model and validators
  • Load in initial data if needed
• Log in to the admin and add one or two users to NotificationSubscription
• Run docker-compose run --rm app python manage.py send_expiration_email
• Confirm that appropriate statuses display in the terminal and all users associated with notifications were sent an email
• Confirm that:
  • if there are licenses expiring, the email says how many and includes a csv with appropriate details
  • if there are no licenses expiring, the email says that, and does not have any attachments

[xmedr]

I've realized that I can get the commands for the cronjob into the Dockerfile, so I'll be adding that shortly!

[xmedr]

@smcalilly Alrighty, I've got changes based off of your suggestions and that article you linked. Ideally, this checks everyday at ~9am for any users who wanted notifications on that day, and sends them an email with licenses expiring within one year's time. It then updates those users' notification dates to next year.

How would testing this work? I would think we could make a user that wants notifications sent tomorrow, but it doesn't look like this app deploys to Heroku.

[xmedr]

What do you think of this line? Is it worth leaving this out of the earlier for sub in subscribers loop on line 69?

The hope was that users wouldn't get their notification dates updated if the command fails midway. That way if this command were to run again, they could still have a chance to be included. Lmk if that makes zero sense haha.

[smcalilly]

that's a good thing to consider. this works fine, though it might be more idiomatic django to wrap it in a transaction: https://medium.com/@shivanikakrecha/transaction-atomic-in-django-87b787ead793

but that would be a lot of logic in the transaction — if you did that, then you could encapsulate more of the email sending code into functions that are called within the transaction.

[smcalilly]

@xmedr nice work.

How would testing this work? I would think we could make a user that wants notifications sent tomorrow, but it doesn't look like this app deploys to Heroku.

Great question. I'm not sure what the URL is for the staging or prod app. Let me see if we can track that down.

[smcalilly]

that's a good thing to consider. this works fine, though it might be more idiomatic django to wrap it in a transaction: https://medium.com/@shivanikakrecha/transaction-atomic-in-django-87b787ead793

but that would be a lot of logic in the transaction — if you did that, then you could encapsulate more of the email sending code into functions that are called within the transaction.

[smcalilly]

you could move this to line 65 and skip the rest of the logic if len(near_expired) == 0. idk if we want to send an email if there are 0 or not?

[xmedr]

i can definitely do that! i figured having a "hey don't worry about it, everything's okay" notification could be useful, but not sending the email in the first place would make it so those people would have one less thing to deal with, which could also be a nice QOL thing

[smcalilly]

gotcha — this works as it is then!

[xmedr]

@smcalilly Cool, I've got the command wrapped in a transaction and did some refactoring! I tried to encapsulate as much as I could in helper functions.

Transactions seem super useful, how often do you find yourself using them? I had read that they're a bit taxing to run so I don't imagine they're used everywhere all the time.

[smcalilly]

@xmedr yeah you need to be careful with it because it locks up the database. In this case, if the mgmt command was running at the same time somebody was trying to change the date in the admin, then they wouldn't be able to change the date until the transaction is committed. I'm sure there are other ways that make transactions resource intensive but I'm at the edge of my knowledge.

I don't use them a ton. I use them whenever there's any logic that has a higher likelihood to fail, like sending an email or requesting a third-party API. Or, if there's a critical operation where data integrity is the most important thing — a classic example is withdrawing money from a bank account. These are only some of the examples I'm most familiar with, I'm sure there are more use cases.

Based on your implementation in this PR (where you were asking about how to handle this in case the email sending fails), it seems like you have the right instinct for when to use it or not.