feat(reports): implement report emailing #2310

jniles · 2017-11-28T15:39:50Z

This commit implements the email dropdown on reports as a first step
towards automatic email reporting. The automatic email reports will
depend on a scheduler proposed in #2308.

The email functionality depends on having a mailgun account. You should
put your mailgun API key and domain in the .env.development file. Some
defaults for this project have been filled out.

Partially addresses #1688 and #1766.

Fig 1: The Email Link Dropdown

Fig 2: The Email Modal. It defaults to the current user's email address

Fig 3: Email in my inbox

DedrickEnc

@jniles

I made some comments and asked some questions to know more about the code. I waiting for your clarification.

Well done!
Thanks!

DedrickEnc · 2017-12-01T14:33:14Z

client/src/i18n/fr/report.json

@@ -131,7 +131,8 @@
      "BACK":"Revenir en arriere",
      "OPTIONS":"Options",
      "PREVIEW":"Aperçu",
-      "VIEW_ARCHIVE":"Archives"
+      "VIEW_ARCHIVE":"Archives",
+      "EMAIL_HELP_TXT" : "L'e-mail ci-dessus recevra le rapport en pièce jointe. Le serveur doit avoire une connexion Internet pour que l'e-mail soit livré avec succès."


Aie ... avoir surelly a typo ~~avoire~~

DedrickEnc · 2017-12-01T14:43:15Z

client/src/modules/reports/baseReports.service.js

@@ -63,6 +65,25 @@ function BaseReportService($http, Modal, util, Languages) {
      .then(util.unwrapHttpResponse);
  }

+  function emailReport(uuid, email) {
+    var url = '/reports/archive/'.concat(uuid, '/email');
+    return $http.put(url, { address : email })


Why a PUT method? We are not updating the report (resource) right?

Yeah, I needed a body in the request and only PUT or POST requests have a body. I just chose PUT. Do you think POST is better?

Yes, let use POST if possible

DedrickEnc · 2017-12-01T14:52:03Z

client/src/modules/reports/modals/reports.email.js

+  vm.reportName = options.reportName;
+
+  vm.params = {
+    email : Session.user.email,


DedrickEnc · 2017-12-01T14:52:25Z

client/src/modules/reports/modals/reports.email.js

+  };
+
+  vm.submit = function submit(EmailForm) {
+    if (EmailForm.$invalid) { return 1; }


Why not just return?

It was to satisfy ESLint. We should probably turn that rule off - it causes more harm than good, I think.

Yes you are right

DedrickEnc · 2017-12-01T15:11:38Z

server/controllers/report.js

+
+  lookupArchivedReport(uuid)
+    .then(report => {
+      debug(`#emailArchived(): sending ${report.label} to ${address}.`);


Is there a difference between debug and winston?

Good question! Winston uses "error levels" (winston.log(), winston.error(), winston.warn()), which debug simply namespaces logs by module. It's like having console.log() statements that you can turn on and off.

This is advantageous because you can turn all logs off except the ones you care about, or you can filter logs based on what module it came from. For example, here we log only email logs by using DEBUG=mailer and it will only print the debug() calls that are in the mailer module. That way, if there are errors in the email script, we will be able to easily track them down in production by either filtering through the logs or turning off all logs except the email logs.

This issue gives more of a detailed overview about why debug() works better for our needs than winston.

Thank you for the explanation

DedrickEnc · 2017-12-01T15:13:16Z

server/controllers/report.js

+      const parameters = {
+        filename,
+        date,
+        user : report.display_name,


What is the difference between the user and the requester?

The user created the report originally and archived it. The requester is the person asking for the email to be sent. The AG might create the report, but the MD might request it to be sent somewhere.

DedrickEnc · 2017-12-01T15:27:07Z

server/lib/mailer.js

+ */
+exports.email = function email(address, subject, message, options = {}) {
+  debug(`#email() sending email ${subject} to ${address} with options %j`, options);
+  return processAttachments(options.attachments)


No method handle error, so it will be handled outside?

Yes, it is handled here.

I think we will want to pass back errors to the module that requests the email to be sent. That way, the parent module can decide what to do - throw the error or retry the email or take another action.

jniles · 2017-12-03T12:24:04Z

@DedrickEnc, Thanks for the review. I've fixed the typo - good catch!

I need your feedback on the POST vs PUT issue. Either one works - which makes the most sense to you?

DedrickEnc · 2017-12-03T13:04:43Z

@jniles
Every thing is fine, let just change the PUT method to POST one. And you can run bors.

This commit implements the email dropdown on reports as a first step towards automatic email reporting. The automatic email reports will depend on a scheduler proposed in IMA-WorldHealth#2308. The email functionality depends on having a mailgun account. You should put your mailgun API key and domain in the .env.development file. Some defaults for this project have been filled out. Partially addresses IMA-WorldHealth#1688 and IMA-WorldHealth#1766.

jniles · 2017-12-03T13:47:24Z

Thanks!

bors r+

2310: feat(reports): implement report emailing r=jniles a=jniles This commit implements the email dropdown on reports as a first step towards automatic email reporting. The automatic email reports will depend on a scheduler proposed in #2308. The email functionality depends on having a mailgun account. You should put your mailgun API key and domain in the .env.development file. Some defaults for this project have been filled out. Partially addresses #1688 and #1766. ![emailactivatedindropdown](https://user-images.githubusercontent.com/896472/33365180-9add6914-d4e7-11e7-99ad-0bd9c5ce499e.PNG) _Fig 1: The Email Link Dropdown_ ![emailmodal](https://user-images.githubusercontent.com/896472/33365181-9b0fa924-d4e7-11e7-9336-1051137986d0.PNG) _Fig 2: The Email Modal. It defaults to the current user's email address_ ![emailreceivedwithattachment](https://user-images.githubusercontent.com/896472/33365182-9b590790-d4e7-11e7-9821-78c2aad0356f.PNG) _Fig 3: Email in my inbox_

bors · 2017-12-03T14:15:49Z

Build succeeded

continuous-integration/travis-ci/push

jniles requested a review from DedrickEnc November 28, 2017 15:39

jniles force-pushed the feat-simple-email-of-reports branch from c5a66e1 to df068fd Compare November 30, 2017 09:34

DedrickEnc reviewed Dec 1, 2017

View reviewed changes

jniles force-pushed the feat-simple-email-of-reports branch from df068fd to 000e0c2 Compare December 3, 2017 11:29

jniles force-pushed the feat-simple-email-of-reports branch from 000e0c2 to f5c47f5 Compare December 3, 2017 13:42

jniles force-pushed the feat-simple-email-of-reports branch from f5c47f5 to df86542 Compare December 3, 2017 13:44

bors bot merged commit df86542 into IMA-WorldHealth:master Dec 3, 2017

jniles deleted the feat-simple-email-of-reports branch December 4, 2017 08:29

jniles mentioned this pull request Dec 4, 2017

(feature request) Email the administrative accounts when a new user is added #1579

Open

jniles mentioned this pull request May 24, 2019

Add monthly, trimesterial, and annual reports automatic report sending #3745

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(reports): implement report emailing #2310

feat(reports): implement report emailing #2310

jniles commented Nov 28, 2017 •

edited

Loading

DedrickEnc left a comment

DedrickEnc Dec 1, 2017

DedrickEnc Dec 1, 2017

jniles Dec 3, 2017 •

edited

Loading

DedrickEnc Dec 3, 2017

DedrickEnc Dec 1, 2017

DedrickEnc Dec 1, 2017

jniles Dec 3, 2017

DedrickEnc Dec 3, 2017

DedrickEnc Dec 1, 2017 •

edited

Loading

jniles Dec 3, 2017

DedrickEnc Dec 3, 2017

DedrickEnc Dec 1, 2017

jniles Dec 3, 2017

DedrickEnc Dec 1, 2017

jniles Dec 3, 2017

DedrickEnc Dec 3, 2017

jniles commented Dec 3, 2017

DedrickEnc commented Dec 3, 2017

jniles commented Dec 3, 2017

bors bot commented Dec 3, 2017

feat(reports): implement report emailing #2310

feat(reports): implement report emailing #2310

Conversation

jniles commented Nov 28, 2017 • edited Loading

DedrickEnc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jniles Dec 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DedrickEnc Dec 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jniles commented Dec 3, 2017

DedrickEnc commented Dec 3, 2017

jniles commented Dec 3, 2017

bors bot commented Dec 3, 2017

Build succeeded

jniles commented Nov 28, 2017 •

edited

Loading

jniles Dec 3, 2017 •

edited

Loading

DedrickEnc Dec 1, 2017 •

edited

Loading