Pass kwargs from AbstractUser.email_user() to send_mail()

[jug]

Add fail_silently=False to User.email_user and pass it to send_mail.

[John-Scott Atlakson]

Added patch so email_user() passes kwargs to send_mail.

[Aymeric Augustin]

Change UI/UX from NULL to False.

[Aymeric Augustin]

Change Easy pickings from NULL to False.

[Klaas van Schelven]

Just weighing in on a ticket that seems to be living in limbo for three years at this point:

This should either be 'design decision needed', or be quickly resolved, since it seems rather trivial to me. What's needed to move this forward?

Personally, I don't think passing **kwargs around is the pinnacle of elegance. Are there more kwargs that we're interested in, or is it really just fail_silently?

As a final thought, if we consider this a desirable feature, maybe fail_silently should be customizable in more cases. Specifically, it's not easy to customize the password_reset view to have a different value for fail_silently without subclassing and completely copy/pasting PasswordResetForm. Maybe that's a separate ticket?

[Tim Graham]

The patch needs to be updated to apply cleanly. It also needs tests (doesn't look like email_user is tested at all) - probably in django/contrib/auth/tests/test_models.py. The doc in the current patch (docs/topics/auth.txt) has moved to docs/ref/contrib/auth.txt) and a mention in the release notes is also needed.

[Susan Tan]

@timo, The only aspect that is the most confusing to me is the unit test. How do I test that the email sent to a user was a successful event? I see that email_user doesn't return anything from a quick read of the code. In principle, I picture that the new test inside UserManagerTestCase(TestCase) in the tests/test_models.py should look like this:

def test_send_email(self):
        keyword_args = {"fail_silently":False, "auth_user":None, "auth_password":None,
              "connection":None, "html_message":None}
        sucesss_indicator = UserManager.send_email(subject="This is subject",
            message="This is a message", from_email="susan1@domain.com",
            recipient_list="susan2@domain.com", **keyword_args)
        self.assertEqual(sucesss_indicator, "<expected output of success>")

What do you think? I'm not sure what the success_indicator is, the expected output for send_email function.
Can you point me in the right direction? Are there any other testing considerations that I missed?

[Tim Graham]

email_user() is a method on AbstractUser not UserManager, otherwise the test looks like a reasonable start.

You can read about testing email here: ​https://docs.djangoproject.com/en/dev/topics/testing/overview/#email-services

[Susan Tan]

@timo, Thanks for the reference. I've written the test. I'm used to running tests on the django/tests directory. But in this case, the test is in django/contrib/auth/tests/test_models.py. How do I test this file? The auth/tests direcotry does not have a runtests.py.

I've referred to this doc for running tests:
​https://docs.djangoproject.com/en/1.5/intro/contributing/#running-your-new-test

Below is the new test:

@skipIfCustomUser
class AbstractUserTestCase(TestCase):
    def test_send_email(self):
        keyword_args = {"fail_silently":False, "auth_user":None, "auth_password":None,
              "connection":None, "html_message":None}
        AbstractUser.send_email(subject="Subject here",
            message="This is a message", from_email="from@domain.com",
            recipient_list="to@domain.com", **keyword_args)

        # Test that one message has been sent.
        self.assertEqual(self.assertEqual(len(mail.outbox), 1))

        # Verify that the subject of the first message is correct.
        self.assertEqual(mail.outbox[0].subject, 'Subject here')

[Tim Graham]

runtests.py django.contrib.auth should work to run the test.

Regarding the test itself, I would format the dictionary a bit differently (space after colon, each key/value pair on a separate line).

kwargs = {
    "fail_silently": False,
    ...
}

Also, I don't think this test class requires the @skipIfCustomUser decorator. You'll need to instantiate an AbstractUser rather than calling send_email directly as it isn't a classmethod.

[Susan Tan]

I have a PR ready for your review and testing: ​https://github.com/django/django/pull/1469

Feedback is welcome, as always. I've commented out the email_user function in django.contrib.auth models.py and ran the new test, which failed. Then, I un-commented out the email_user function and the tests passed, both unit test and full test suite.

[Tim Graham]

Comments on PR.

[Susan Tan]

All comments incorporated. See updated PR: ​​https://github.com/django/django/pull/1469 Let me know if there's still any more feedback.

[Tim Graham <timograham@…>]

In 71c491972eecae8783cf46e69fac7e5f9f83fc59:

Fixed #11400 -- Passed kwargs from AbstractUser.email_user() to send_mail()

Thanks Jug_ for suggestion, john_scott for the initial patch,
and Tim Graham for code review.