-
Notifications
You must be signed in to change notification settings - Fork 2
Replace cTms email rows with Braze #1
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
Conversation
basket/news/tasks.py
Outdated
| None, | ||
| {"email_id": token}, | ||
| { | ||
| "country": update_data["country"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A number of custom attributes are blocklisted. Some of them have columns in cTms but are blocked in Braze. We should check in and see if this is intentional as there won't be a 1 to 1 match without them.
basket/news/backends/braze.py
Outdated
|
|
||
| return self._request(BrazeEndpoint.CAMPAIGNS_TRIGGER_SEND, data) | ||
|
|
||
| def set_subscription_status(self, email, newsletters): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Braze expects subscription_group_ids for this call instead of the short key. This means we'll need to pass those along to django as well in the subscription process.
There doesn't seem to be an endpoint to get subscription group ids. I guess they'll be hardcoded or entered in the database? Too soon to be able to tell.
They're added in to basket as braze_id in the news_newsletters table.
|
|
||
|
|
||
| class Newsletter(models.Model): | ||
| braze_id = models.UUIDField( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no mechanic to get a list of subscription groups from Braze so we need to store them somewhere. I think this makes the most sense.
basket/news/backends/braze.py
Outdated
| """ | ||
| Set subscription status for the specified newsletters | ||
| https://www.braze.com/docs/api/endpoints/subscription_groups/post_update_user_subscription_group_status/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heads up, you linked to v1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heads up, you linked to v1
Good catch I wouldn't have remembered that.
|
Reminder to myself: check that the basket_token is being recorded correctly as an attribute in braze when testing this PR (we'll need it for the news/confirm endpoint). UPDATE: Have checked, it is. |
| {"email_id": new_user.get("email", {}).get("email_id")}, | ||
| { | ||
| "basket_token": token, | ||
| "country": update_data["country"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may not exist -- only email and newsletters are required in the request. You can submit an API request without the country or the language fields, which causes the code to error out on this line (update_data won't have a country property).
| "basket_token": token, | ||
| "country": update_data["country"], | ||
| "email_format": "H", | ||
| "language": update_data["lang"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue as 'country' above
| } | ||
| for newsletter in Newsletter.objects.all(): | ||
| newsletter.braze_id = braze_mapping.get(newsletter.slug, None) | ||
| newsletter.save(update_fields=["braze_id"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heads up, we might be replacing the values in the existing vendor_id column instead for this. Please, ping someone to find out was what was decided around that when you're back from vacation. https://silverorange.slack.com/archives/C09C0L23JG1/p1759151093820139?thread_ts=1758726117.837289&cid=C09C0L23JG1
| braze.track_user( | ||
| data["email"], | ||
| None, | ||
| {"email_id": new_user.get("email", {}).get("email_id")}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking into this, I don't think this will work when we remove ctms, since the email id is coming from that ctms add. It's fine for now, just something to address later on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That and the ctms generated ID won't work yes. We will have to come back to this later on.
Pre-generate tokens/email_ids for fxa tasks
No description provided.