-
Notifications
You must be signed in to change notification settings - Fork 4
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
No sms messageset notification #181
No sms messageset notification #181
Conversation
changes/tasks.py
Outdated
messageset = messagesets_rev[re.sub("^whatsapp_", "", short_name)] | ||
messageset = messagesets_rev.get(re.sub("^whatsapp_", "", short_name)) | ||
|
||
if not messageset: |
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.
I'm worried that we're including too much here. It would rather be better that if we forget about any messageset that's whatsapp only, that it results in an exception, rather than sending the message that is very specific for 1-2 messaging, and us not noticing that anything is wrong.
So I think we should rather check if the message is the specific messageset that we're concerned about here, rather than checking for any missing messageset.
changes/tasks.py
Outdated
{ | ||
"to_identity": change.registrant_id, | ||
"content": text, | ||
"channel": "JUNE_TEXT", |
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 should be sent over WhatsApp
changes/tasks.py
Outdated
"metadata": {}, | ||
} | ||
) | ||
change.data["error"] = "No whatsapp available" |
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.
I don't think that this is descriptive enough. Maybe something like "WhatsApp-only messagesets cannot be switched to SMS"
changes/tasks.py
Outdated
@@ -572,14 +572,38 @@ def switch_channel(self, change): | |||
) | |||
|
|||
elif change.data["channel"] == "sms" and "whatsapp" in short_name: | |||
# Change any WhatsApp subscriptions to SMS | |||
sbm_client.update_subscription(sub["id"], {"active": False}) |
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 means that we'll no longer be deactivating the service info messageset, which we still want to do.
Mom who is on 1-2 Message Set (WA) and tries to switch herself to SMS will get the following autoresponse: “We notice that you have been receiving MomConnect msgs on WhatsApp for children between 1 - 2. Messages for children between 1 - 2 are only available on WhatsApp - switching to SMS means you will not receive any messages. You can stop your MomConnect messages completely by replying “STOP”. NB: The system MUST NOT switch the mom to SMS, she can either stay on WA or initiate an opt out herself.