-
Notifications
You must be signed in to change notification settings - Fork 447
test: Check if email is sent for new mentorship relation requests #536
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
Conversation
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.
@gaurivn I requested some changes, let me know if you have any doubt or disagree with any of the changes :)
app/api/email_utils.py
Outdated
@@ -149,3 +149,5 @@ def send_email_new_request(user_sender, user_recipient, notes, sender_role): | |||
) | |||
subject = "Mentorship System - You have got new relation request" | |||
send_email(user_recipient.email, subject, html) | |||
|
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.
can you undo these changes? just to keep the changes to test module?
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.
can you remove these changes? I can merge them but if you could take these off as well. This seems like style format changes, that can be done in another PR
Sure @isabelcosta thank you, I will make the changes asap. |
@isabelcosta , I made the changes, kindly review them and please tell if I have to make further changes. |
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.
there is left just some indentation fixes @gaurivn
@@ -45,6 +53,8 @@ def test_fail_send_request_bad_mentor_id(self): | |||
headers=auth_header, content_type='application/json', | |||
data=json.dumps(test_payload)) | |||
self.assertEqual(404, actual_response.status_code) | |||
self.assertEqual(1, len(outbox)) |
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.
is this well indented?
@@ -62,9 +72,45 @@ def test_fail_send_request_bad_user_id(self): | |||
headers=auth_header, content_type='application/json', | |||
data=json.dumps(test_payload)) | |||
self.assertEqual(400, actual_response.status_code) | |||
self.assertEqual(1, len(outbox)) |
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.
is this well indented?
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.
@isabelcosta , I have made the required indentation changes after removing unnecessary commits, please review and suggest changes if any.
app/api/email_utils.py
Outdated
@@ -149,3 +149,5 @@ def send_email_new_request(user_sender, user_recipient, notes, sender_role): | |||
) | |||
subject = "Mentorship System - You have got new relation request" | |||
send_email(user_recipient.email, subject, html) | |||
|
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.
can you remove these changes? I can merge them but if you could take these off as well. This seems like style format changes, that can be done in another PR
@isabelcosta , when I checked in gedit editor, it shows proper indentation. I thought it was because of mixing tabs and spaces. I had removed the extra lines under email_utils.py, got a bit confused with the commit conflicts. Can you please check once again and tell if there are any changes I can make. |
@isabelcosta , it is fixed now, I used spaces instead of tab, please review. |
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.
Can you please check again code indentation @gaurivn :)
@@ -14,6 +15,12 @@ | |||
class TestSendRequestApi(MentorshipRelationBaseTestCase): | |||
def setUp(self): | |||
super(TestSendRequestApi, self).setUp() | |||
self.notes_example = 'description of a good mentorship relation' |
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 think this is not well indented and that is why TravisCI is failing
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 indented it properly, replaced tab with proper spaces.
@@ -29,9 +36,10 @@ def test_fail_send_request_bad_mentee_id(self): | |||
headers=auth_header, content_type='application/json', | |||
data=json.dumps(test_payload)) | |||
self.assertEqual(404, actual_response.status_code) | |||
self.assertEqual(1, len(outbox)) |
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.
outbox
isn't defined here.
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.
@gaurivn can you please define how to get the outbox from the user email
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.
@isabelcosta @BadduCoder, can I remove the outbox line
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.
@gaurivn I did a quick search on google to try to figure out how to use the outbox
concept, and I found this in the official documentation of Flask-Mail: https://pythonhosted.org/Flask-Mail/#unit-tests-and-suppressing-emails
Can you see if you can fix the outbox
usage in this test, before deciding to not use this?
This issue goal is about using something in the code to figure out if the email is being sent, and I think this can be done through this outbox
concept
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.
@gaurivn outbox
isn't imported/defined anywhere, which is causing the travis ci failure. Fix that first, and for indentation (maybe use some linter)?
@gaurivn can you please make the necessary changes? |
@gaurivn closing due to inactivity |
Description
Testing whether email is sent for valid mentorship relation requests and whether email is not sent for invalid mentorship relation requests with 2 added unit tests. Changed the version of MarkupSafe (to 1.1.1) in requirements.txt.
Fixes #432
Type of Change:
Delete irrelevant options.
Code/Quality Assurance Only
How Has This Been Tested?
Tests were added to check if email was sent for new mentorship relation requests.
Checklist:
Delete irrelevant options.
Code/Quality Assurance Only