Fix accepted mail sending when present in eventsignups POST
Created by: temparus
Ensures that an accepted email is sent to the user when an admin created an event signup directly with accepted = true
.
Fixes #415 (closed).
Merge request reports
Activity
requested review from @jzumthurm
Created by: NotSpecial
Review: Changes requested
Thank you for your work!
I have some feedback regarding the test:
- I would recommend to not merge this test with an existing test, to make clearer what is being tested.
- The test should maybe go into
test_emails.py
-- I would expect tests regarding event emails there. - So far the tests only checks it any email was sent. I think we should be more explicit:
- Either check that the correct email is sent (see email tests for an example).
- Or use
mock
and just ensure thatnotify_signup_accepted
was called correctly.
requested review from @jzumthurm
removed review request for @jzumthurm
requested review from @jzumthurm
234 """Test that an accepted email is sent when an admin has added a 235 signup manually.""" 236 event = self.new_object('events', spots=1, selection_strategy='fcfs', 237 allow_email_signup=True) 238 239 user = self.new_object('users') 240 241 self.api.post('/eventsignups', data={ 242 'user': str(user['_id']), 243 'event': str(event['_id']), 244 'accepted': True 245 }, token=self.get_root_token(), status_code=201).json 246 247 mail = self.app.test_mails[0] 248 for field in mail.values(): 249 self.assertTrue('None' not in field) Created by: temparus
Well, the test case with the None values in the templated string fields is pretty obvious to me. Nevertheless I just added a comment there which additionally explains it in English.
So to explain it again with an example: A confirmation email for the event signup of event XYZ should be sent. This is a prepared message containing placeholders. So the event name is a placeholder which gets replaced by 'XYZ'. In case the event name is None for some weird reason, the event name then listed in the message is 'None'.
Created by: codecov[bot]
Codecov Report
All modified and coverable lines are covered by tests
Comparison is base (
ece574e
) 94.76% compared to head (3b45865
) 94.77%.Additional details and impacted files
@@ Coverage Diff @@ ## master #416 +/- ## ========================================== + Coverage 94.76% 94.77% +0.01% ========================================== Files 75 75 Lines 4315 4329 +14 ========================================== + Hits 4089 4103 +14 Misses 226 226
Flag Coverage Δ unittests 94.77% <100.00%> (+0.01%)
Flags with carried forward coverage won't be shown. Click here to find out more.
View full report in Codecov by Sentry.
Have feedback on the report? Share it here.requested review from @jzumthurm
252 253 self.api.post('/eventsignups', data={ 254 'email': 'a@example.com', 255 'event': str(event['_id']), 256 'accepted': True 257 }, token=self.get_root_token(), status_code=201).json 258 259 # accepted email and confirmation email should be sent. 260 self.assertIs(len(self.app.test_mails), 3) 261 for mail in self.app.test_mails[1:]: 262 for field in mail.values(): 263 self.assertTrue('None' not in field) 264 265 self.assertTrue( 266 ('accepted' in mail['subject']) or 267 ('confirm_email' in mail['text'])) mentioned in issue #493 (closed)
requested review from @jzumthurm
requested review from @jzumthurm