Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Brevo webhook updates #387

Merged
merged 8 commits into from
Jul 25, 2024
Merged

Conversation

originell
Copy link
Contributor

@originell originell commented Jul 21, 2024

rel #385

As talked about in the related issue, I've updated the mappings. I also added tests and docs.

Furthermore, I tried to make the code comment I missed ( "request" event) stand out more, by moving all the comments from inline to above their related entry. I hope this is OK.

Very much looking forward to your comments!

@originell
Copy link
Contributor Author

A question: while testing the new mappings, I noticed that Brevo had some problems with the proxy_open event. Hence, I contacted their support and I've received this as an answer:

The team has confirmed that they needed to make a few database changes so that this event type could be added to our collections.
We have made changes for you and you should now receive “unique_proxy_open” for the first/unique opening.
For subsequent openings, you will receive “proxy_open” events.

Now, I am asking myself: should I also supply a mapping for unique_proxy_open as well?

It is undocumented and I've only gotten access to this by asking support.

@medmunds
Copy link
Contributor

Thanks, your updates look great.

should I also supply a mapping for unique_proxy_open as well?

Yes. (I think.)

I'd guess that proxy_open and unique_proxy_open will parallel opened and unique_opened, so mapping both will be necessary to receive all the proxy open events.

We ended up normalizing both of the original "opened" to Anymail's OPENED, and added this paragraph to the docs about how to choose. See #344 if you're interested in the history.

# Sadly, this is not well documented in the official Brevo API documentation.

I may get that printed on a T-shirt 😄. (Not to pick on Brevo: pretty much every ESP's documentation for webhooks is some combination of incomplete and just plain inaccurate.)

@originell
Copy link
Contributor Author

Thanks for the feedback! I've now added the suggested changes.

Let me know if something could be done better!

@medmunds
Copy link
Contributor

medmunds commented Jul 25, 2024

That's great, thanks. I updated the docs to combine some of the earlier information about open events with your new information.

(Also, nice work debugging Brevo's proxy open behavior and the missing unique_proxy_open event!)

@medmunds medmunds merged commit d05f448 into anymail:main Jul 25, 2024
61 checks passed
@originell
Copy link
Contributor Author

originell commented Jul 25, 2024

Perfect! Thanks for merging those. Sounds way better that way.

nice work debugging Brevo's proxy open behavior

Haha thanks. But that was more people work than debugging work. Their support was very helpful though. Wasn't more than 2-3 emails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants