isomerpages / isomercms-backend

A static website builder and host for the Singapore Government
5 stars 1 forks source link

fix(email): log mail status after reasonable delay #803

Closed caando closed 1 year ago

caando commented 1 year ago

Problem

Postman API send mail post request to endpoint /transactional/email/send returns status code 201 indicating the request has been created, but does not guarantee that the email will be sent and delivered.

Closes IS-135

Solution

After a reasonable delay, verify with Postman that the mail has been delivered. If otherwise, log the status of the email to be tracked.

Improvements:

Notes: This change will cause Users.spec.ts to encounter warnings for remaining asynchronous operations after the tests are complete. This is because MailClient.ts has a separate routine for verification of email statuses which has a delay of 1 minute. Waiting for that long will cause the tests to timeout. useFakeTimers() have been experimented with, but behaviour seem to be buggy and cause all tests to fail. Hence, am currently sticking with the warning and letting the tests pass.

caando commented 1 year ago

@kishore03109

From my previous convo with Postman, they mentioned that they will tell us if an email is blacklisted in the first API call itself. Can you also add a logger to state xxx@blah.gov.sg is blacklisted? this way it would be easier for the oncall to quickly tell if an email has been blacklisted or not!

From Postman API:

However, there are two ways you will be informed if a recipient is on the blacklist:

  1. In our current setup, during your API call to our system, we will check if the recipient is on the blacklist. If the recipient is on the blacklist, we will return an error message indicating that the recipient is on the blacklist. However, to scale our system, we plan to change this behavior in the future, so we advise against relying on this behavior.
  2. You can query an email by its ID using this API endpoint. If the email was not sent because the recipient was on the blacklist, this will be indicated in the errorCode field of the response.

It seems that they are intending to remove the feedback from first post api call. I will instead check for the errorCode on verification. What are your opinions on this?

seaerchin commented 1 year ago

could you rebase on top of develop/merge develop into this branch? got extra + conflicts