Auto email reports by using cron job #3767

mbayopanda · 2019-06-19T09:06:57Z

This PR add a new feature of sending report automatically by email.

In this PR, we have :

In the enterprise settings an option to enable or disable this feature of emailing
Create a component to add in reports modules for having this feature on the report module
Added this feature in :
- Balance Report
- Bilan Report
- Compte Resultat Report
- Compte d'exploitation Report

hound · 2019-06-19T09:07:21Z

server/lib/mailer.js

+        // asynchronously load the file and add it as as an attachment
+        return fs.readFile(attach.path)
+          .then(file =>
+            new mailgun.Attachment({


Expected no linebreak before this expression implicit-arrow-linebreak

jniles

@mbayopanda, I've not gotten a chance to test this, but have written some of my thoughts below.

jniles · 2019-06-24T12:29:32Z

client/src/js/components/bhCronEmailReport/bhCronEmailReport.js

+      report_url : $ctrl.reportUrl,
+      has_dynamic_dates : 0,
+    };
+    $ctrl.isFeatureEnabled = Session.enterprise.settings.enable_auto_email_report;


Let's name this isEmailFeatureEnabled or something a little more precise. Even though we are in the $scope of a component, it will be helpful in the future to know what feature is enabled if the application throws an error.

jniles · 2019-06-24T12:30:02Z

client/src/js/components/bhCronEmailReport/bhCronEmailReport.html

+        </bh-yes-no-radios>
+
+        <div class="text-right">
+          <button data-method="save-cron-report" type="submit" class="btn btn-primary" translate>


Nice data-method name!

jniles · 2019-06-24T12:32:02Z

client/src/js/components/bhCronEmailReport/bhCronEmailReport.js

+  }
+
+  function submit(cronForm, reportForm) {
+    if (reportForm.$invalid) {


I don't think you need to add this as a parameter. Isn't it available as $ctrl.reportForm? If so, is there a reason not to just use $ctrl.reportForm?

jniles · 2019-06-24T12:35:03Z

server/models/schema.sql

+  `entity_group_uuid` BINARY(16) NOT NULL,
+  `cron_id` SMALLINT(5) NOT NULL,
+  `report_id` SMALLINT(5) NOT NULL,
+  `report_url` VARCHAR(200) NOT NULL,


Do we need to store the report_url? Can't we just look it up from the report table?

I recommend we just look it up - it will be much easier to make database migrations that way. We'll only need to change the URL in one place.

jniles · 2019-06-24T12:43:35Z

server/lib/moment.js

+/**
+ * this class helps to get start date and end date of a period
+ */
+const moment = require('moment');


I advise that we do two things with this library:

rename it to something like bhmoment or something similar. The name moment will be misleading to inexperienced developers.

Write some unit tests! If we call it with the date '2015-01-01', what should .day() return? What about .week()? etc...

jniles · 2019-06-24T12:56:45Z

client/src/i18n/en/cron.json

+    "LAST":"Last Send",
+    "NEXT":"Next Send",
+    "PLEASE_FILL_REPORT_FORM":"Please fill correctly the report form",
+    "PLEASE_FILL_CRON_FORM":"Please fill correctly the email form",


A similar comment to the one above: "The email form isn't filled out correctly. Please check the values and resubmit."

jniles · 2019-06-24T12:57:27Z

client/src/i18n/en/cron.json

+    "DYNAMIC_DATES":"Dynamic dates",
+    "EMAIL_SENT_SUCCESSFULLY":"Email sent successfully",
+    "SEND":"Send",
+    "DELETE":"Delete"


Doesn't DELETE exist somewhere already? Like FORM.BUTTONS.DELETE? Can we just use that?

jniles · 2019-06-24T12:59:31Z

client/src/i18n/en/enterprise.json

-      "ENABLE_AUTO_STOCK_ACCOUNTING_HELP_TEXT" : "Enabling this feature will write stock movement transactions into the posting journal in real time. It requires all inventory accounts to be correctly configured."
+      "ENABLE_AUTO_STOCK_ACCOUNTING_HELP_TEXT" : "Enabling this feature will write stock movement transactions into the posting journal in real time. It requires all inventory accounts to be correctly configured.",
+      "ENABLE_AUTO_EMAIL_REPORT_LABEL" : "Enable sending automatic reports by email",
+      "ENABLE_AUTO_EMAIL_REPORT_HELP_TEXT" : "Enabling this option gives access to the reporting configuration for automatic emails"


This text is probably better: "Enabling this option allows users to configure schedules to automatically send reports to a list of email addresses."

jniles · 2019-06-24T13:00:27Z

client/src/js/components/bhCronEmailReport/bhCronEmailReport.html

+        </bh-input-text>
+
+        <!-- group -->
+        <div class="form-group">


I don't think you need this form-group. Looks like you have included it in the bhEntityGroupSelect component.

jniles · 2019-06-24T13:01:12Z

client/src/js/components/bhCronEmailReport/bhCronEmailReport.html

+        </div>
+
+        <!-- frequency -->
+        <div class="form-group">


It doesn't look like you need this form-group <div>. You have included it in the bhCronSelect component.

…ling

mbayopanda · 2019-07-06T10:19:56Z

@jniles could you review this PR again please

jniles

Looks good to me! I'll fix the migration script, then we can merge it.

jniles · 2019-07-08T10:58:16Z

test/server-unit/bhMoment.spec.js

+const startTime = '00:00:00.000';
+const endTime = '23:59:59.999';
+
+const date = new BhMoment(dateString);


jniles · 2019-07-08T11:03:43Z

test/integration/cronEmailReport/cronEmailReport.js

+
+  it('GET /cron_email_reports list registered cron email reports', () => {
+    return agent.get('/cron_email_reports')
+      .send(record)


Do you really need this .send()? I don't think it applies to GET requests.

jniles · 2019-07-08T11:08:13Z

bors r+

3767: Auto email reports by using cron job r=jniles a=mbayopanda This PR add a new feature of sending report automatically by email. In this PR, we have : - In the enterprise settings an option to enable or disable this feature of emailing - Create a component to add in reports modules for having this feature on the report module - Added this feature in : - Balance Report - Bilan Report - Compte Resultat Report - Compte d'exploitation Report ![PCfDwQk18n](https://user-images.githubusercontent.com/5445251/59752269-f2941580-9279-11e9-8088-7c18982c973e.gif) Co-authored-by: mbayopanda <mbayopanda@gmail.com> Co-authored-by: Jonathan Niles <jonathanwniles@gmail.com>

bors · 2019-07-08T11:31:29Z

Build succeeded

continuous-integration/travis-ci/push

3853: Release v1.2.0 r=jniles a=jniles 🚀 Let's release bhima (currently at 1.1.1) Changelog: * chore: use release-it for releases (f4efd33) * chore: rename migration path (37fe3b4) * chore: dump dependencies before release (492ac19) * Merge #3846 (c2f27c8) * Merge #3849 (7a41977) * bug(Cash box translation) - Fix untranslation key for user deactivated (b0f4fbe) * Merge #3836 (f20d459) * refactor(Make Exchange Rates human readable) - Addition of the minimum value of the exchange rate to prevent the exchange rate equal to zero - Show exchange rates less than 0.1 as a fraction for example 1/1630 instead of 0.0006134 (c21e24a) * Merge #3829 (28fdd28) * test: run client unit tests on AppVeyor (a1c8968) * chore: refactor testing code (64b3c96) * test: remove server-unit from TravisCI (0357ec2) * test: add appveyor to bors (b790a45) * ci: add initial appveyor support (c235a97) * Merge #3840 (8d3010a) * fix: bump unit ids in the migration file (9bf73ee) * chore(package): update eslint-config-airbnb-base to version 14.0.0 (6fbde6a) * Merge #3834 (8774b09) * handle negative values in stock report (57380f1) * Merge #3833 (217d0d1) * add translation in employee report (aee91f6) * Merge #3832 (3578b7f) * fix(Calcul Sum Distributed) - Around the sum distributed for escape error when percent as decimal (30dbc83) * Merge #3799 (852b220) * clean code (95f1e93) * improvement(Employee Standing) - clean the representation of tabSQL - Capitalize creditoropeningbalance (01893e9) * Sanitaze code (7268cad) * improvement(Employee Standing) - Fix and Upgrate the translation key - Adding opening balance until a date from - Adding new function openingBlanceCreditor with parameters like creditorUuid and datefrom (3588ff8) * Sanitaze code (b914bca) * refactor(Employee Standing) - Refactor and Sanitaze code (e115407) * improvement(Improve Employees Standings report) - Add the possibility to visualize the employees standings by range of date, by the adding of component date by bh-date-interval - Add filter by range of date (f4c4774) * Merge #3827 (f1a7e6f) * Merge #3828 (19f0753) * fix(tests): no async forEach in merge patients (c4b257a) * feat(report) debtor summary report (a835fb3) * chore: update depends (b78ab2a) * Merge #3825 (1326302) * Merge #3794 (7ae875f) * (cron job) add cron email componet to main reports (438b1ac) * improvement(Invoicing Bug) - Restoring the old debtor balance request and modifying the hasCreditorBalance calculation by analyzing whether the balance is greater than 0.01 (b289a4e) * Merge #3815 (d115442) * improve(Monthly Balance) - Rename Monthly Balance instead Monthly Balance Analysis - Sanitaze code and refactor Report Design (0ae1f6a) * Merge #3820 (64497cc) * improvement(MonthlyBalance) - Sanitaze code and improve report and translate key (58cb1d0) * Sanitaze code (1167990) * Sanitaze code (9455a3d) * feature(Monthly Balance Analysis) - Add new report for Analysis Monthly Balance of accounts (06d9f60) * Merge #3801 (87edd98) * remove toggle required (7c0f583) * Improve and sanitaze code (b613b29) * fix (Invoicing Bug) - resolution of the problem when calling the Debtors.balance function with rounding to two ranks after the decimal point of the total credit and debit values (1edfd7b) * rename cashboxId to cashboxAccountId (f895692) * fix component require condition for validation (00869ad) * Enhance caution filter and fix cashflow by service report (a6bfc8b) * Merge #3821 (2973736) * fix stock adjustment deletion (ab9db1d) * Sanitaze html form (3494521) * improvement(Cash Flow Reports) - Adding Possibilty to view the detailed cashflow report (946e278) * Merge #3813 (21d19d3) * chore(package): update cz-conventional-changelog to version 3.0.0 (6c941a5) * Merge #3808 (34fe9fc) * Merge #3809 (f5d7fb2) * chore(package): update commitizen to version 4.0.0 (2dfc6cb) * chore(package): update gulp-if to version 3.0.0 (6af28e8) * Merge #3797 (cda7c86) * chore(package): update karma-chrome-launcher to version 3.0.0 (d3be817) * Merge #3795 (0d5adeb) * fix cron table on migration (8ad64ba) * Merge #3767 (84b5af0) * Merge remote-tracking branch 'upstream/master' into auto-email-reporting (ada6cd6) * Merge #3775 (ce8af95) * Merge #3785 (f50b9c6) * fix e2e tests for cron emailing (e39aa2a) * Merge #3777 (e30235b) * fix(Payslip) - Restaure the management of off days in payslip - fix the displaying of daily rate for Holidays periods and offdays - Exted the periode of definition of offdays (713c4bf) * Merge #3782 (14ae5ba) * fix: .snyk & package.json to reduce vulnerabilities (b3c8d1a) * Merge #3779 (47ba601) * Merge #3780 (ad5115c) * fix integration test (cd531cb) * chore(package): update lockfile yarn.lock (3ef4a2d) * chore(package): update del to version 5.0.0 (72c68d7) * fix label index length (45e6dc3) * improve(Employee Standing) - Adding option to include medical care provided to the employee - Update Report, display Employee Standing Status in Report closes #3778 (331d9c1) * fix column length (bf3259c) * fix: restaure old file (e2e1e4a) * feature(Bhima User) - Complete Bhima User Manual (18a0514) * fix entity parent (852144d) * update report id length (842b864) * fix: migration script typo (bf1157a) * fix translations and unit test for bhMoment (8109ab7) * fix migration file (e1edfba) * dev environment (bb2b5ba) * fix ui and integration tests for cron email report (d68d383) * instant email send for reports (2bbd820) * ohada_balance_sheet, ohada_income_expense, exploitation, balance emailing (ea4fac0) * Form reset after submission and cron job fixing (7ab8b44) * handle deletion and job stop (2101165) * balance report by email and use of async await (b9cec59) * sending email (af4b449) * cron job for email report (54fc906) * load report details (c8c9115) * simple crud for cron email report (46d7a34) * contact group management (f3c040f) * feature(Bhima User Manual) - References accounts User Manual - Fee Center User Manual - Payroll User Manual (0aacc6a) * Merge #3772 (78a657a) * fix: rubrics config tests (c31beee) * fix: flaky role management test (90a6fcb) * Merge #3768 (a18c9f5) * Merge #3770 (c65a8d5) * chore: bump dependencies (96207a2) * Merge #3724 (b244207) * fix purchase reference (5f0dc18) * Merge branch 'master' of https://github.com/IMA-WorldHealth/bhima into report-stock-entry (efc5e43) * chore(package): update lockfile yarn.lock (c0f80d7) * chore(package): update eslint to version 6.0.0 (e90a2e7) * fix stock sheet report (ef42f3b) * Capitalize report description and use of reportDetails correctly (0af0ec8) * stock entry report (9b7f65c) * Merge #3762 (d2bc25a) * Merge branch 'master' into fix-depot-path (75ca9e2) * Merge #3760 (c13c5a4) * fix depot path (18d2eee) * feat(component) bhperiod, use period translate_key as label (d061ecc) * Merge #3759 (b7cc0d3) * Merge #3736 (f95255b) * Merge #3751 (30ea36d) * add warning message for employees patients merge (3236947) * Merge branch 'master' of https://github.com/IMA-WorldHealth/bhima into merge-patient-records (ff4f7da) * chore(package): update lockfile yarn.lock (aef66b2) * chore(package): update mochawesome to version 4.0.0 (21fe8dd) * Merge #3752 (9880ae8) * Merge #3757 (fc69b92) * update action link column (200d0a7) * updates integration and e2e tests (ce256bc) * Split hospitalization indicators by projects (18d659b) * count services by project (995a746) * add project in service (8a9b6bd) * Merge #3755 (752723a) * Merge #3756 (480f324) * fix: typos in migration script setup (3c9ed3e) * Reset Numbers of vouchers in E2E test (f3345be) * Restore number of vouchers in Ui grid (d9319af) * Reset defaut number of vouchers (dde4bb0) * test(Voucher Registry) - Update number of vouchers (6b7683a) * improvement(Payroll Configuration) - Update the max length of abbreviation of rubrics - remove unused require - Before we had to prevent the edition of all the headings which were taxes, but it can be made that there are taxes which is not calculated expressed as a percentage, to allow the edition of this kind of tax, it would be enough to exclude the only tax which is expressed as a percentage neither and which is not nor éditable reason why we had excluded the IPR - closes #3754 (b70ef8d) * improvement(Payroll) - Sanitaze code remove unused require employees - Use the date to instead the current days for commitment of paiement in accounting (c62c98f) * Merge #3753 (80e2607) * fix(invoice): primary key collision invoicing_fees (df82dad) * fix selection list (b6681a6) * Merge #3743 (1a3d919) * reload page before each tests (a939788) * fix price list report (b4471b5) * integration tests and e2e tests of patients merge fetaure (93bd991) * merge patients tool (a6ca2de) * merge patients (5dc2a0b) * chore(package): update merge-stream to version 2.0.0 (e8005ec) * Merge #3732 (54212b3) * Merge #3733 #3734 (b2eec0b) * chore: combine date/username lines on receipt head (52fdbdb) * Merge #3735 (b68bd3f) * Merge #3737 (4886914) * Merge #3739 (f2b3983) * feat(unpaid invoice) add filter by service (e14ed21) * fix indicator report missing value (4bfac9a) * fix: parse <head> contents from XLS renderer (f15481a) * fix: dismiss fiscal loading indicator once loaded (6b5bd8f) * fix: account reference dropdown header links (f40a358) * fix: display correct currency in account footer (9642e6f) * Merge #3730 (c33886f) Co-authored-by: Jonathan Niles <jonathanwniles@gmail.com>

kwilu added the size: XL label Jun 19, 2019

hound bot reviewed Jun 19, 2019

View reviewed changes

mbayopanda requested a review from jniles June 19, 2019 12:25

jniles reviewed Jun 28, 2019

View reviewed changes

mbayopanda added 14 commits July 2, 2019 00:57

contact group management

f3c040f

simple crud for cron email report

46d7a34

load report details

c8c9115

cron job for email report

54fc906

sending email

af4b449

balance report by email and use of async await

b9cec59

handle deletion and job stop

2101165

Form reset after submission and cron job fixing

7ab8b44

ohada_balance_sheet, ohada_income_expense, exploitation, balance emai…

ea4fac0

…ling

instant email send for reports

2bbd820

fix ui and integration tests for cron email report

d68d383

dev environment

bb2b5ba

fix migration file

e1edfba

fix translations and unit test for bhMoment

8109ab7

mbayopanda force-pushed the auto-email-reporting branch from 71a76f1 to 8109ab7 Compare July 1, 2019 23:58

mbayopanda added 6 commits July 2, 2019 09:08

update report id length

842b864

fix entity parent

852144d

fix column length

bf3259c

fix label index length

45e6dc3

fix integration test

cd531cb

fix e2e tests for cron emailing

e39aa2a

jniles approved these changes Jul 8, 2019

View reviewed changes

Merge remote-tracking branch 'upstream/master' into auto-email-reporting

ada6cd6

bors bot merged commit ada6cd6 into Third-Culture-Software:master Jul 8, 2019

jniles mentioned this pull request Aug 20, 2019

Release v1.2.0 #3853

Merged

mbayopanda deleted the auto-email-reporting branch September 27, 2019 08:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auto email reports by using cron job #3767

Auto email reports by using cron job #3767

mbayopanda commented Jun 19, 2019

hound bot Jun 19, 2019

jniles left a comment

jniles Jun 24, 2019

jniles Jun 24, 2019

jniles Jun 24, 2019

jniles Jun 24, 2019

jniles Jun 24, 2019

jniles Jun 24, 2019

jniles Jun 24, 2019

jniles Jun 24, 2019

jniles Jun 24, 2019

jniles Jun 24, 2019

mbayopanda commented Jul 6, 2019

jniles left a comment

jniles Jul 8, 2019

jniles Jul 8, 2019

jniles commented Jul 8, 2019

bors bot commented Jul 8, 2019

Auto email reports by using cron job #3767

Auto email reports by using cron job #3767

Conversation

mbayopanda commented Jun 19, 2019

hound bot Jun 19, 2019

Choose a reason for hiding this comment

jniles left a 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

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

mbayopanda commented Jul 6, 2019

jniles 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 commented Jul 8, 2019

bors bot commented Jul 8, 2019

Build succeeded