-
-
Notifications
You must be signed in to change notification settings - Fork 433
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
add sms provider seven #1242
add sms provider seven #1242
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1242 +/- ##
=======================================
Coverage 99.36% 99.37%
=======================================
Files 147 148 +1
Lines 20555 20636 +81
Branches 3663 3672 +9
=======================================
+ Hits 20425 20506 +81
Misses 121 121
Partials 9 9 ☔ View full report in Codecov by Sentry. |
This is a really great PR; thank you! just a few small tests should get you the rest of the coverage you need and we can merge this right away. Great work! 👍 |
# Our expected url(privacy=True) startswith() response: | ||
'privacy_url': 'seven://a...a/15551232000', | ||
}), | ||
('seven://{}/15551232000'.format('a' * 25), { |
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.
Add the following and you'll be at 100% test coverage:
('seven://{}/?to=15551232000'.format('a' * 25), {
# target phone number using to=
'instance': NotifySeven,
# Our expected url(privacy=True) startswith() response:
'privacy_url': 'seven://a...a/15551232000',
}),
('seven://{}/15551'.format('a' * 25), {
# target phone number invalid
'instance': NotifySeven,
# Our call to notify() under the hood will fail
'notify_response': 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.
Hi, thanks for your help! I just pushed the changes.
Just to confirm that it is working for you (before i merge). Also, i prepared the following Wiki page for your review: here |
Description:
This PR adds seven as a SMS provider.
New Service Completion Status
%global common_description
Checklist
flake8
)Testing
Anyone can help test this source code as follows:
Thanks for looking into it and please be easy on me, this is my first PR for this project :-)