Skip to content

Conversation

@gautam1858
Copy link

Adding whatsapp notification

Adding whatsapp
@VictorSanh
Copy link
Contributor

Hello @gautam1858,
I might be missing something: why is the whatsapp_sender file exactly the same as sms_sender.py? If it's the same, then using the same call should be sufficient (I haven't checked how to send messages through WhatsApp through python).
Also, please test and check your code for obvious errors (for instance, the file whatsapp_sender should have a .py extension). I also prefer having all the changes in the same pull request (cf PR #33 ).
Victor

@gautam1858
Copy link
Author

gautam1858 commented Jan 8, 2020

@VictorSanh Have made the necessary changes that you have asked for, its similar to sms but here I have added prefix WhatsApp so that its compatible for WhatsApp 🤗

@VictorSanh
Copy link
Contributor

Ok, I understand how it works now.

I would have a more simple/direct approach:

def whatsapp_sender(account_sid: str, auth_token: str, recipient_number: str, sender_number: str):
    prefix = 'whatsapp:'
    return sms_sender(account_sid=account_sid, recipient_number=prefix+recipient_number, sender_number=prefix+sender_number)

The code is exactly the same modulo this prefix, so no need to duplicate a whole bunch of code (plus you have to make sure all the cases+exceptions are covered).

Also, before merging, you also need to make the modifcations to the README, __init__.py and __main__.py. For instance, you can have a look at this complete pull request (#36) I merged a few days ago.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants