Cloudkibo / KiboPush

0 stars 1 forks source link

send weekly email button is not working in the user section operational Dashboard #8908

Open arveenkumar55 opened 4 years ago

arveenkumar55 commented 4 years ago

Describe the bug Currently, when I try to click on the send weekly email button it is not working in the user section operational Dashboard. Also when I click on send weekly email button it moves to upward.

To Reproduce Steps to reproduce the behavior:

  1. Go to Operational Dashboard
  2. Go to User Section
  3. Click on send a weekly email button
  4. See error

Expected behavior It should send a weekly email.

Front End or Backend End issue? Backend How was the issue found the automated test or manual test? Manual

When was this issues last working? It is not working from start

Screenshots Screenshot from 2020-06-12 14-52-50

Paper Trail logs There were no paper trail logs here

Browser logs There were no browser logs here

Sentry Links and Screenshots There were no sentry links here

Desktop (please complete the following information):

Additional context Add any other context about the problem here.

saniasiddiqui commented 4 years ago

I have looked into this issue. Email is working fine on local and staging. However on production I am not receiving email and when I looked into the papertrail it shows that accounts server is crashing whenever I send an email: image So I will have to replicate this on local to find out the exact cause by importing the data from production on my local db. Will discuss with @sojharo to see logs for Accounts (we donot have that in papertrail and ill need credentials to view them).

saniasiddiqui commented 4 years ago

Below are findings in this issue because of which we are having problems in production

  1. This weekly email is sent to all users when we click on the button 'Send Weekly Email' in production.
  2. In the logic, we have a loop that runs through all users, calculates their weekly summary and emails to the user. So we have right now almost 400 users on production and this call is made 400 times i.e sendgrid API is called 400 times. This clogs the service and we are getting ECONNRESET in paper trail. I tried to limit the users by sending email to super users only and email was being sent.
  3. To scale this up, I have researched and found the solution in the use of lib: @sendgrid/mail. The lib makes use of sendgrid ability to send multiple emails in one request. The limit is 1000. For 1000 emails we can make one request to send grid. image Full documentation :https://www.twilio.com/blog/sending-bulk-emails-3-ways-sendgrid-nodejs
  4. To implement the logic discussed in point 3, we will re write the code to send weekly emails. Current code will also be modified in the process, which is right now not modular and difficult to debug. @sojharo @ImranBinShoukat @jekram
jekram commented 4 years ago

Great Analysis - Let's discuss this tomorrow 👍

ImranBinShoukat commented 4 years ago

I had discussed this with @saniasiddiqui yesterday. She should make the above changes and reduce the no. of api calls we are making to SendGrid. This will improve the performance.

saniasiddiqui commented 4 years ago

@arveenkumar55 please test on staging. For testing purpose email will be sent to super users only. Once it is tested we can remove the super user flag.

arveenkumar55 commented 4 years ago

Summary shows incorrect data in email of the body. It is showing 0 activity in all cases.

saniasiddiqui commented 4 years ago

@AnishaChhatwani this is complete. Please test thanks

AnishaChhatwani commented 4 years ago

The summary is correct now. I checked it on papertrail because emails are not being received due to #8960

will test this again when this once #8960 is resolved

jekram commented 4 years ago

Please do not send any email - without a review - this logic has caused Sendgrid account to get blocked multiple times.

How many emails will be sent out when #8690 is fixed?

I thought @arveenkumar55 has communicated this to @AnishaChhatwani and @saniasiddiqui

saniasiddiqui commented 4 years ago

Yes.. this was communicated by @arveenkumar55. For testing purpose, we have added the logic that email will be sent to super users only. There are 12 super users in staging so on testing this email will be sent to 12 users.

When the logic will be on production. Email will be sent to all users. We have 400 users.

saniasiddiqui commented 4 years ago

This logic sends email to all users. We have 400 users right now. And for sendgrid there is a restriction of 100 emails in a day for free account as discussed in issue #8960. Please discuss what we can do about it. @sojharo @ImranBinShoukat

sojharo commented 4 years ago

Please do in parts, do first 80 today.

saniasiddiqui commented 4 years ago

I have done some research here and discussed with @ImranBinShoukat and @sojharo.

With current logic the button is operational dashboard 'Send Weekly Email' sends activity summary to all users. Currently we have approx 400 users. image

Now as we got to know after suspension of our account from sendgrid #8960 that for free account there is a limit of 100 emails/day. So with the current account we cannot send weekly email to all users on a single day.

As an alternative we researched APIs of MailChimp. Mandrill is basically the add on on mailchimp which allows transactional (customized) emails to customers. Mandrill is a paid add-on which is available for a paid account on Mailchimp (we already have that). The cheapest block of Mandrill credits is $20/month. Also mandrill has no API for sending bulk emails i.e. if we have to send 400 emails we will need to call their API 400 times. image

Bulk email functionality is present in sendgrid, which we have implemented in this task. According to Sendgrid pricing we can have up 50k emails/ month in $14.95 image

As far as sending emails in chunks is considered, we will require a rather detailed design for that. We need to decide how many users should receive email in a day so that 100 emails/day limitation is not crossed. Also we will need to track for the status for all users, whom we have sent the email, who is remaining. And then reset their status every week. @sojharo @ImranBinShoukat

jekram commented 4 years ago

@saniasiddiqui There are other reasons for sending emails in chunks. If we send all we may not have the capacity to respond to the customers. What is wrong with a UI where we select x to y? 1 to 80, 81 to 160 or 1 to xxxx. If we buy the upgraded service even then we may blast all. Let me know what the effort would be?

ImranBinShoukat commented 4 years ago

There are two issues:

  1. How will remember the context. How would we know to whom we need to send the email and to whom we have already sent?
  2. How will be determine what number would not let us hit the 100 emails limit? If we set it to be 80 then only 6-7 more people can create account on KiboPush on that day because we send three emails if a new customer creates an account on KiboPush.
    • Joining email to user
    • Account creation email to me, @sojharo and @jekram
    • Email verified email to user If more than 7 people try to create an account that day then they will not receive the emails.
sojharo commented 4 years ago

I have looked into mailchimp for this requirement. The type of emails that we send in this issue are the emails which contain data in their body (i.e. stats about account usage). This data is different for different customers. The stats show their kibopush usage to customers in weekly email.

The mailchimp subcription that we have only sends emails which are same for all customers i.e. campaign emails. However, if we want to send data in email body which may differ from customer to customer then we should use mailchimp addon called "Transactional Email". Formerly, it was called Mandrill. It allows us to put data in email bodies using api call and then send personalised emails to our customer with their own data. While sendgrid has this by default as the main focus of sendgrid is to send such dynamic emails.

Here is link to mailchimp's add on called transactional email.

https://mailchimp.com/features/transactional-email/

We will have to purchase this service of mailchimp separately as they have separate pricing for this.

https://mailchimp.com/pricing/transactional-email/

We have also ongoing communication to switch to mailchimp's mandrill here: https://github.com/Cloudkibo/KiboPush/issues/2519

I have also reviewed and looked into both apis of sendgrid and mailchimp mandrill. The only difference between them is pricing and bulk api support.

I couldn't find option of bulk email in mandrill, however there is bulk email api in sendgrid.

We may go for either of both and as we grow, we will have to purchase subscription for either sendgrid or mailchimp mandrill.

Mandrill has good integration with mailchimp as it is product by mailchimp itself. It would be good option to go for mandrill as in future.

On sending email in parts i.e. to first 80 to 100, we can do this with mailchimp as well. We may not need to send email to all of our customers. Many of them may have created account just for testing purpose and many may have abandoned after creating accounts. We can put in UI the option to "send weekly usage email to top 80 customers".

jekram commented 4 years ago

Please much is the cost?

saniasiddiqui commented 4 years ago

MailChimp Standard account has Mandrill add-on that is used to send transactional* emails MailChimp Standard account: $14.99/month Mandrill basic account: $20 for 25000 emails/month

SendGrid Cost: $14.95/month for 50,000 emails /month , For Free : 100 emails/day

*Transactional emails: Email sent from one sender to one recipient, usually related to account activity or a commercial transaction. Examples include password reminder emails, purchase confirmations, and personalized product notification