kblincoe / QualOpt_SE701

2 stars 15 forks source link

#36 Back-end processes bounced emails. #114

Closed will-molloy closed 6 years ago

will-molloy commented 6 years ago

This is a partial solution to #36; I still need to do the front-end. I want to merge this now because other contributors (@pulkitkalra, @Karim-C) are working on the same files for issues #26 and #28.

Changes: Back-end StudyService now checks for bounced participant email addresses after an invitation has been sent out and returns a json array of any bounced address to the front-end. This works by opening a POP3 session for the users email and checking for any message from Gmail mail delivery subsystem. It then processes these messages for "Failure to sent" or similar and extracts the failed address. Another check is made that the failed addresses actually came from this sent study. The result is then included in response from the existing endpoint; POST /api/studies/send.

Note: This solution is specific to Gmail. I read a universal approach is to forward bounced emails with the sufficient information but I could not get this to work, see http://cephas.net/blog/2006/06/09/using-apache-james-and-javamail-to-implement-variable-envelope-return-paths/ I don't think it works because mail servers now protect from spoofing the 'from' address. This probably comes under #91.

Integration test:

  1. Login as admin (admin:admin).
  2. Create some participants (some with invalid emails e.g. invalid123123123123123123@gmail.com). (invalid address will throw an exception; this is another issue.)
  3. Create a study and add the participants.
  4. Send the study.
  5. Console.log (f12) should show the bounced address; mvnw debug log should too.

Unit tests: I've created several tests in /test/java/org/project36/qualopt/service/StudyServiceIntTest and src/test/java/org/project36/qualopt/web/rest/StudyResourceIntTest.java. This involves mocking a mail box using javamail-mock2-fullmock to mock the Gmail mail delivery subsystem. The tests invoke the StudyService#sendInvitationEmail() method (black box tests) and should be sufficient sanity tests for @pulkitkalra and @Karim-C to run when they do their issues.

Note: Don't remove the test scope from javamail-mock2-fullmock in pom.xml. This means the mock inbox will be used in production and emails will no longer be sent.

will-molloy commented 6 years ago

@Karim-C, @AprajitGandhi requesting review when you have time :)

pulkitkalra commented 6 years ago

the tests are great, thank you for those @wilmol 👍

Karim-C commented 6 years ago

I have a lot of merge conflicts StudyService.java file due to changes I made for #28. Other than that no other conflicts. I'll be merging my code and making a PR once this PR is merged. I may need your input later this week to resolve the conflicts.

will-molloy commented 6 years ago

@kblincoe is it OK that my implementation is specific to Gmail and depends on their mail sub delivery system not changing?

I did a bit of research and an outdated solution is to forward failed emails but I couldn't get this to work (I think Google is protecting from spoofing emails). Another method I found was using an API to verify an emails mailbox exists (e.g. https://neverbounce.com/), however they seem to require payment or have a limited number of emails you can process.

kblincoe commented 6 years ago

Hmm Gmail specific is not ideal. Does this mean the person sending the message has to have Gmail or the receiver? I thought all of the messages were being sent from a QualOpt account, so couldn't we checked for returned messages there?

will-molloy commented 6 years ago

Only the sender has to be Gmail. This is when users send their study at the moment a single Gmail account is used, when #91 is fixed it could be any email.

kblincoe commented 6 years ago

OK, sure.

will-molloy commented 6 years ago

@softeng-701 all approved ready for merge.

softeng-701 commented 6 years ago

@wilmol Please squash your commits, thanks!

will-molloy commented 6 years ago

@softeng-701 squashed

softeng-701 commented 6 years ago

@wilmol sorry for the wrong spelling, thank you! merged now!