search

sklarsa / django-sendgrid-v5

An implementation of Django's EmailBackend compatible with sendgrid-python v5+
MIT License
318 stars 54 forks source link

Fixes outlined in pull #93 #95

Closed alichass closed 3 years ago

alichass commented 3 years ago

_build_sg_personalization "to" arg now changed to optional; either it or existing_personalizations must be set.

Prevent using .personalizations without tos set.

Prevent unnecessary resetting of tos when personalization is already constructed.

No longer provide tos if not set for personalizations. This is to prevent every personalization from being sent to every recipient in msg.to

Added/Changed some comments/docstrings to reduce ambiguity

(I closed the previous pull by accident)

alichass commented 3 years ago

Hi I added the test to check the errors raised (the only new functionality I added really - as far as I can tell the existing tests cover the new logic) but for some reason when I run nosetests -c nose.cfg locally it doesn't seem to run the new tests (I checked this by adding an arbitrary assert that should have failed)

sklarsa commented 3 years ago

That's odd, it works for me. I added assert 1 == 0 at the start of your tests on your branch (locally):

======================================================================
FAIL: test_build_personalization_errors (test.test_mail.TestMailGeneration)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/steven/source/github/sklarsa/django-sendgrid-v5/test/test_mail.py", line 600, in test_build_personalization_errors
    assert 1 == 0
AssertionError

The test output was: Ran 17 tests in 2.054s

Looking at some sample tox output from the github actions... Ran 17 tests in 0.544s

So I think they're being run in CI, which passed, so I'll merge.

Thank you!