As some might have seen in Copenhagen (Day) or Vigo, I’ve created a package called django-pony-express. Since I got (in my opinion) really good feedback from a bunch of smart people, I’d like to suggest to move some stuff that I’ve built into core.
I think, the test suite might be a great starting point. I’ve create a wrapper for mail.outbox so you can use it like a Django Queryset. (Docs).
My suggestion: html_content = email_test_service.filter(subject='Nigerian prince').get_html_content()
And for assertions:
list_of_emails = email_test_service.filter(subject=subject)
# We expect an email to be sent
self.assertEqual(list_of_emails.count(), previous_email_count + 1)
# Assert content
list_of_emails.assert_body_contains(f"Hola {user.first_name}")
Currently, the functionality lives in a class which you have to instanciate. I’m open to suggestions on how to make this (even) more django-esque.
The code is simple, it works, it’s documented and it’s tested.
I’d like go gather some thumbs-up to check if this is worth following up on.
I am +1 on having get_html_content() or a html_content attribute on EmailMessage, because mail.outbox[0].alternatives[0][0] is not intuitive that this is html content and emails having html content is common.
On some of the helper asserts (such as assert_body_contains which asserts this is both in the text and html content), my main question is on the API. For example:
I think once we get the API down and make sure we have good error messages and docs for these I would be +1 on adding a helper like this as having content in both the text and html part is good practice and a nice battery to add. However, I think we need more voices on what the right API here would be.
On .filter(), I can see that this looks nice but I’m wondering if people will want .get() and .exists() and others if we go down that road. I am -0 currently. Not sure the value gained is worth the effort
Note: we should extend the email service testing topic in Django to include an email with html and the asserts you will want to do here. This can then be updated with get_html_content() / html_content and other helpers when they land.
I’d like to add a warm emote in the general direction here. Yes, +1, let’s improve email testing.
That way is probably more in line with what we have. If we go this route, I wonder if a specialist EmailTestCase class would be worth it? (Pro: keep email related methods in one place, separate from an already long list of methods. Con: do you end up need TestCase and friends anyway to write the actual tests you need? IDK but it’s a thought.)
I not sure though that @GitRon’s suggestion of having the assertions on the helper isn’t the way forward… — It’s a wrapper about the mailbox functionality, only used in tests, and then the filtering and assertion logic lives in the one place, not spread between the new wrapper and the test case.
It’s not the helper’s responsibility to make assertions though. (That’s the tests). So rather than assert_... have methods that return a boolean – .body_contains(...) in the example – and then assert as usual in the test case – either self.assertTrue(list_of_emails.body_contains(...)) for unittest based tests, or assert list_of_emails.body_contains(...) for those folks using pytest.
(Pro there is keeping the email related stuff in the helper class.)
(So many possibilities: we shouldn’t design it too much in committee — any of the options will be fine. )
I’ve had a quick look into multipart/alternative emails as I was worried that emails might have multiple "text/html" alternatives. Apparently the order of attachments is important as they should increase in content “richness”/complexity.
Maybe we should have .alternative_body_contains(..., type=None) which checks the last alternative or the last alternative of a particular type (e.g. type="text/html").
One of the nice things @GitRon added was having the asserts check all the parts, so that you don’t forget to update just one template. Keeping something of that would be good. (Though there doesn’t have to be just one method, of course)
A body_contains() method looks like a good compromise for making it possible to assert on multiple bodies without repetition.
I would be a -1 here. Python already provides many tools for filtering and sorting lists (list comprehensions, sorted(), map(), filter()). Reimplementing the QuerySet API would just be “another” way to do it, perhaps even an unintuitive one because there would be no database query or laziness.
Since it seems the consensus points towards this method, in which class would we put this helper? Just want to make sure that I’ll work in the right direction
Additionally, how would we support other types like… audio that @sarahboyce mentioned. Just document for now that we just check plain text and HTML?
Since it seems the consensus points towards this method, in which class would we put this helper?
Maybe on EmailMultiAlternatives?
On handling other types, I think we should only check text (so text/plain, text/enriched, text/html etc) and skip anything else.
It may not even be a problem but think it’s a good idea to check the MIME type
Ok, so @theorangeone has a further improvement up his sleeve, my improvement to the emailalternatives class and the docs are somewhere on their way to core.
I do feel that some kind of wrapper for the mail.outbox would add genuine value but I can understand @adamchainz argumentation of reinventing the wheel.
So, the question is: Are we done here or do we want to add more improvements apart from the three PRs?
thank you for raising this discussion
but it’s a thought.)
)
