No migration #37
No migration #37
Conversation
email_id and token are equivalent going forward so use the passed in token for both.
basket/news/views.py
Outdated
| lang = user_data.get("lang", "en") or "en" | ||
| email_id = user_data.get("email_id") | ||
| tasks.send_recovery_message.delay(email, user_data["token"], lang, email_id) | ||
| tasks.send_recovery_message.delay(email, email_id, lang, email_id) |
There was a problem hiding this comment.
Shouldn't you just remove the token param here so you don't need to pass email_id twice?
jacobpenny
force-pushed
the
no-migration
branch
from
8765892 to
b7074e0
Compare
We're using the email_id as the token going forward
|
|
||
| # These tasks cannot be placed in basket/news/tasks.py because it would | ||
| # This task cannot be placed in basket/news/tasks.py because it would | ||
| # create a circular dependency. |
There was a problem hiding this comment.
Did you mean to leave this comment here?
There was a problem hiding this comment.
Yes because there is still one task (the alias one)
There was a problem hiding this comment.
Maybe it's worth removing the empty space between that comment and that task then? So it's clear that's what it's about
basket/news/tasks.py
Outdated
| txm = BrazeTxEmailMessage.objects.get_message(message_id, lang) | ||
| if txm: | ||
| user_data = {"basket_token": token, "email_id": email_id} | ||
| user_data = {"basket_token": email_id, "email_id": email_id} |
There was a problem hiding this comment.
Nitpick, but send_confirm_message is using the "token" param and send_recovery_message is using the "email_id" param -- you might want to make them consistent with each other instead?
| expected = { | ||
| "attributes": [ | ||
| { | ||
| "_update_existing_only": False, |
There was a problem hiding this comment.
You can delete this test (test_to_vendor_with_updates_and_no_user_data_in_braze_only_write) -- we no longer have that conditional external id based on braze_only_write
| @param token: basket_token | ||
| @param email: email address | ||
| @param fxa_id: external ID from FxA | ||
| @return: dict, or None if not found |
There was a problem hiding this comment.
Since we're not using the email_id param in braze.get, we should probably get rid of it to avoid even more confusion
2 participants
No description provided.