search

CuriousLearner / django-phone-verify

A Django app to support phone number verification using security code / One-Time-Password (OTP) sent via SMS.
https://www.sanyamkhurana.com/django-phone-verify/
GNU General Public License v3.0
258 stars 61 forks source link

feat(phone_verify): Add Kavenegar backend #34

Open SepehrHasanabadi opened 4 years ago

SepehrHasanabadi commented 4 years ago

Kavenegar is an Iranian sending message platform like twilio. The backend class is created to send single and bulk messages to iranian sim cards. Twilio does not support Iranian sim cards due to any reason.

SepehrHasanabadi commented 4 years ago

@CuriousLearner Decreasing coverage was inevitable! :(

SepehrHasanabadi commented 4 years ago

I've increased test coverage by mocking top layer of send message functions that is clients like: twilio.rest.Client, instead of the functions we had written before.

CuriousLearner commented 4 years ago

Hi @SepehrHasanabadi

Once you're done with the changes, can you please let me know how can I test the Kavenegar API locally?

SepehrHasanabadi commented 4 years ago

Hi @CuriousLearner
Do you wanna test it on your phone?

Hi @SepehrHasanabadi

Once you're done with the changes, can you please let me know how can I test the Kavenegar API locally?

CuriousLearner commented 4 years ago

Hi @SepehrHasanabadi Yes, the idea was for me to test it, but I don't have a number for that country. I will trust if you can ensure that when you install it in a Django project, this works and you indeed get an SMS :) So, let me know.

CuriousLearner commented 4 years ago

@SepehrHasanabadi Hello,

Is your PR ready to review?

SepehrHasanabadi commented 4 years ago

@CuriousLearner hello, I didn't find any manner to test in your phone. If have any idea It is welcome.

CuriousLearner commented 4 years ago

@SepehrHasanabadi I think it won't be possible for me to test. But can you integrate the package with a django project and do a test that this backend works?

CuriousLearner commented 4 years ago

@SepehrHasanabadi I just updated your branch with the latest code that we have. Can you please take a look at the failing tests? Once they pass, please check that you get a message while using this backend.

thanks!

SepehrHasanabadi commented 4 years ago

@CuriousLearner send_bulk_sms in KavenegarBackend does not implement by sending multi single send_sms therefore test_send_bulk_sms method needed to be modified.

gutsytechster commented 4 years ago

@SepehrHasanabadi will it make sense to put the condition if the backend is Kavenegar, then test in some other way? Most probably, you'll mock the Kavenegar API you are using for sending bulk_sms.

It seems you are doing so, just instead of return statement, you could put Kavenegar specific test within the if condition and put the remaining in the else clause.