MailPace: New ESP

[paul-oms]

Hi Anymail team, a fresh PR for https://mailpace.com with verified commits, pynacl optional, and the linting issues fixed. I was unable to run everything locally, so 🤞 this passes the Github Actions this time round.

• [x] Backend
• [x] Webhooks
• [x] Inbound
• [x] Tags
• [x] Tests
• [x] Docs
• [x] Readme/project/tox/workflows etc.

[medmunds]

This is looking pretty good so far. Added a couple of comments above that might help get the tests working. I'll try to get to a closer review over the weekend.

[paul-oms]

Phew, finally got the tests to pass. Let me know what else is needed.

Also, if you need an API key for integration testing, set up an account at https://app.mailpace.com and we'll add you to the free plan (presumably I can't put the API key anywhere for you).

[medmunds]

Nice! Glad you got the tests working.

I created mailpace.anymail.dev as a testing domain. It would be great if you could set it up for free API use.

There's a decision to be made about tracking webhook verification. I think the choices are: A. Signature verification (MAILPACE_WEBHOOK_KEY, pynacl) is optional; if not used, HTTP basic auth (WEBHOOK_SECRET) is "required" (strongly encouraged via Anymail warning message). This is how you've got it documented. B. Signature verification is required. It's an error to use the MailPace tracking webhook without MAILPACE_WEBHOOK_KEY set and pynacl installed. This is easier to implement and test.

For option A, we need to pull in something like this code to control the basic auth warning, and then also rework the webhook tests to cover both scenarios (rather than just skipping all tests when pynacl isn't installed), similar to how the Resend webhook tests handle optional signature validation with the svix package.

Option B would also be fine—it's really up to you. (pynacl is not tied to any proprietary service, seems likely to be maintained for many years, and doesn't have any other dependencies. It wasn't immediately obvious svix fit those criteria, which is why Anymail's Resend webhooks make signature verification with svix optional.) For option B, we just need to add this line, and allow the configuration error if the webhook key is missing.

[paul-oms]

Thanks! I think Option A is the right choice - always better to make 3rd party dependencies optional.

Does this change cover it?

[paul-oms]

I created mailpace.anymail.dev as a testing domain. It would be great if you could set it up for free API use.

Added to our secret free plan ;)

[paul-oms]

All done - there's one point to check above, but I think it's correct as is.

[medmunds]

Was working on integration tests, and noticed there seems to be a problem with commas in "to" display names. Try sending "to": "\"Example, Inc.\" <info@example.com>". I get a 400 response with "errors": { "to": [ "is invalid" ] }.

(No need to go through Anymail—same problem just using curl with json. Probably also affects cc, bcc, and reply_to, though I haven't tested. The "from" field seems OK with commas in display names.)

Trouble with various characters in email display names is pretty common, and normally Anymail implements a workaround involving RFC-2047 encoding. Which is an option here, but maybe you want to look into a fix on your end instead?

[medmunds]

Also, I've added some integration tests. here are some integration tests: test_mailpace_integration.py

You can run them locally with:

export ANYMAIL_TEST_MAILPACE_SERVER_TOKEN=your-server-token
export ANYMAIL_TEST_MAILPACE_DOMAIN=sending.domain.for.your.server.token
python runtests.py tests.test_mailpace_integration 

(These tests won't run in this PR until it's merged, because repository secrets aren't available within PRs from forks. They send real messages, but the destination addresses are all sink emails at anymail.dev.)

(Unfortunately, I can't just add this file to your PR because of a long-standing GitHub bug with forks scoped to organizations. You might want to add me as a collaborator on your django-anymail fork.)

[paul-oms]

Was working on integration tests, and noticed there seems to be a problem with commas in "to" display names. Try sending "to": "\"Example, Inc.\" <info@example.com>". I get a 400 response with "errors": { "to": [ "is invalid" ] }.

(No need to go through Anymail—same problem just using curl with json. Probably also affects cc, bcc, and reply_to, though I haven't tested. The "from" field seems OK with commas in display names.)

Trouble with various characters in email display names is pretty common, and normally Anymail implements a workaround involving RFC-2047 encoding. Which is an option here, but maybe you want to look into a fix on your end instead?

Thanks! This is most likely a bug on our end. I'll look into it and see if we can fix it.

[paul-oms]

You might want to add me as a collaborator on your django-anymail fork.)

Done!

[medmunds]

Thanks, pushed the integration tests.

You might want to add some tests for the features you enabled recently: list-unsubscribe header (but other headers unsupported) and overriding the api token in esp_extra. (I'm happy to add these tests, too.)

I'm still thinking about the best way to map recipient status, will follow up later. Everything else looks good.

[medmunds]

[Rebased to pick up main branch changes]