Skip to content
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

31 sms not sending #32

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

31 sms not sending #32

wants to merge 8 commits into from

Conversation

iamgarcia
Copy link
Collaborator

No description provided.

Alex Garcia added 4 commits April 1, 2022 11:47
- Change function name for sending MMS texts
- Change log messages when sending MMS texts
- Add compression to image prior to sending MMS text
- Import Pillow and IO into notification.py
- Update references of SMS to MMS
@iamgarcia iamgarcia added the bug Something isn't working label Apr 1, 2022
@iamgarcia iamgarcia requested a review from icr-ctl April 1, 2022 21:53
@iamgarcia iamgarcia self-assigned this Apr 1, 2022
@iamgarcia iamgarcia requested review from icr-ctl and removed request for icr-ctl April 18, 2022 18:40
@iamgarcia iamgarcia marked this pull request as draft April 20, 2022 22:44
Copy link
Collaborator

@icr-ctl icr-ctl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

I know we have talked about this in-person a couple of times, but can you sum up the testing you've done to verify it is all working?

@iamgarcia
Copy link
Collaborator Author

There is still another commit or so almost ready for to be pushed into this branch.

To verify it is all working, I added my contact information to the config.yaml.example 5 times to simulate 5 users. I also turned off third-party access to the ScrubDash Google account, which would prevent the system from sending emails and text messages using the account. This will in theory trigger warnings which are visible in the terminal. These warning report that the MMS texts and emails weren't able to send, along with the proper error code and a more detailed description for the error code.

Currently, the way the email system was written leads to all of the emails to be sent out at once, so the terminal will only show a warning to the user in regard to not being able to send out the emails if that is the case. However, since the MMS system sends each message out separately, it will report which phone numbers it was not able to send texts to. Upon testing, these two cases were shown to be true for each of the 5 "users".

Then, I turned on the third-party access to the Google account and the emails and texts were sent out successfully with no issues. Since they were successful, the logging information pertaining to emails and texts are only visible via log.debug().

I also utilized a fake Hollywood number to test the MMS feature. When the account has third-party access disabled, the terminal shows a warning about not being able to send a text to the number due to third-party access being disabled. However, when the account has third-party access enabled, it has the behavior of a successful number. When I printed out what the response that aiosmtplib API gives, there was no issue in the response. Below are the two responses:

Valid phone number: ({}, '2.0.0 OK 1650912262 y126-20020a62ce84000000b0050d223013b6sm8718457pfg.11 - gsmtp')
Invalid phone number: ({}, '2.0.0 OK 1650912264 19-20020aa79113000000b00505d5d15d80sm12640169pfh.14 - gsmtp')

Having said that, when I go to inbox of the Google account, the text message email says "Address not found. Your message wasn't delivered to fakephonenum@mms.cricketwireless.net because the address couldn't be found, or is unable to receive mail." That said, the API behaves as intended. Thus, this is an issue that might need addressing later on if we want to be able to differentiate valid phone numbers from invalid phone numbers.

- Add dictionary of the sender credentials to be used for SMTP
- Add helper method that creates a text message object and returns it
- Add exception handling to MMS sending
- Add exception handling to email sending
- Add verbose output to the terminal for email and MMS sending
@iamgarcia iamgarcia marked this pull request as ready for review April 25, 2022 19:56
@iamgarcia iamgarcia requested a review from icr-ctl April 25, 2022 19:56
@iamgarcia iamgarcia linked an issue Apr 25, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants