Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Links in the email notifications are not working #26

Closed
sonytrinhcvn opened this issue Mar 4, 2020 · 15 comments
Closed

Links in the email notifications are not working #26

sonytrinhcvn opened this issue Mar 4, 2020 · 15 comments
Assignees

Comments

@sonytrinhcvn
Copy link
Collaborator

We got this email when click Unsubscribe & Manage your setting
https://e.dev.ins.unee-t.com/?url=https%3A%2F%2Fcase.dev.ins.unee-t.com%2Fnotification-settings&id=ut_notification_message_new-64&email=trunghieu912003%40gmail.com

@franck-boullier franck-boullier changed the title Fix email links Links in the email notifications are not working Mar 4, 2020
@franck-boullier
Copy link

@sonytrinhcvn can we add a list of:

  • all the emails that have problematic urls
  • all the URLs that are problematic in these emails.
    This will help us test everything when we have implemented the fix.

@franck-boullier franck-boullier self-assigned this Mar 4, 2020
@franck-boullier
Copy link

The code to generate the links in the emails is located in the folder
https://github.com/Unee-T-INS/frontend/tree/master/imports/email-templates.

There are several files that are using a variable ROOT_URL.
@sonytrinhcvn can you help me list these files in that GH issue please.

This variable ROOT_URL is defined as an environment variable in the files aws-env.[STAGE]

@franck-boullier
Copy link

The issue:

In the email, the links generated are something like
https://e.dev.ins.unee-t.com/?url=https%3A%2F%2Fcase.dev.ins.unee-t.com%2Fnotification-settings&id=ut_notification_message_new-64&email=trunghieu912003%40gmail.com
instead of
https://case.dev.ins.unee-t.com/notification-settings&id=ut_notification_message_new-64&email=trunghieu912003%40gmail.com

The code somehow:

  • includes an unnecessary prefix in the email: https://e.dev.ins.unee-t.com/?url=https%3A%2F%2F
  • replace / with %2F

We have 2 solutions to fix the issue:

Quick and dirty:

  • hard code the URLs instead of using variables.

Pros:

Fast

Cons:

This will make it harder/almost impossible t use automated deployment anymore

Proper fix:

  • Understand how we can remove the additional code

@sonytrinhcvn
Copy link
Collaborator Author

sonytrinhcvn commented Mar 4, 2020

@sonytrinhcvn can we add a list of:

  • all the emails that have problematic urls
  • all the URLs that are problematic in these emails.
    This will help us test everything when we have implemented the fix.

Here you are:

  • case-assignee-updated
  • case-new-message
  • case-updated
  • case-user-invited
  • new-case-assigned

@sonytrinhcvn
Copy link
Collaborator Author

The code to generate the links in the emails is located in the folder
https://github.com/Unee-T-INS/frontend/tree/master/imports/email-templates.

There are several files that are using a variable ROOT_URL.
@sonytrinhcvn can you help me list these files in that GH issue please.

This variable ROOT_URL is defined as an environment variable in the files aws-env.[STAGE]

We have some files:

  • case-assignee-updated
  • case-new-message
  • case-updated
  • case-user-invited
  • new-case-assigned
  • report-pdf-shared
  • unit-user-invited
  • helper
  • email-invite
  • logger

@franck-boullier
Copy link

More information:

When we redeploy the code, we can update the ROOT_URL~ in the file aws-env.[STAGE]`

Today this variable is built from other variables. This might cause some unexpected behavior.

The Test:

in the DEV environment: replace the current value
export ROOT_URL=https://case.$STAGE.$DOMAIN on

export ROOT_URL=https://case.$STAGE.$DOMAIN

with
export ROOT_URL=https://case.dev.ins.unee-t.com
in the file https://github.com/Unee-T-INS/frontend/blob/master/aws-env.dev

  • Re-deploy
  • test:
    • create a case
    • invite someone
    • check if links are working.

franck-boullier added a commit that referenced this issue Mar 4, 2020
for more information, see #26 (comment)
@franck-boullier
Copy link

What was done:

Hotfix (c2a4d03) done to test hypothesis described in #26 (comment)

Next step:

Test what are the values for the links generated in the emails.
See if we have fixed the issue or improved the situation.

@franck-boullier
Copy link

franck-boullier commented Mar 4, 2020

More information:

The URLs are generated using code like
url: url.resolve(process.env.ROOT_URL, '/notification-settings')

url.resolve:

a built-in mechanism in Node JS.

From the documentation:

The url.resolve(from, to) is a builtin method of class URL that resolves a target URL relative to a base URL.
Syntax:

url.resolve(from, to);

Where,
from: (type:String) The base URL being resolved against.
to : (type:String) The “href” URL being resolved.

process.env.

Is also a built-in mechanism in Node JS.

The process.env global variable is injected by the Node at runtime for your application to use and it represents the state of the system environment your application is in when it starts. For example, if the system has a PATH variable set, this will be made accessible to you through process.env.PATH which you can use to check where binaries are located and make external calls to them if required.

@sonytrinhcvn
Copy link
Collaborator Author

sonytrinhcvn commented Mar 4, 2020

Test 1:

  • Create a new policy
  • My relationship to the policy: The Insurance Agent
  • Create a new case
  • Assign this case to The Insurance Agent

Expect result

  • The person assigned to the case receives an email - PASSED
  • The email includes a link to the case - PASSED the link was [https://case.dev.ins.unee-t.com/invitation?code=AHfKzw8xtqrw74IhPjVsS1Ja]
  • The link to the case sends the user to that case in InsureChat - PASSED

The source code for this email is in the file https://github.com/Unee-T-INS/frontend/blob/master/imports/util/email-invite.js.
The code that is use could generate the url is

Please click on <a href=${url.resolve(process.env.ROOT_URL, `/invitation?code=${accessToken}`)}>this link</a> to get more information and join the discussion with ${invitorUsername || 'him'}.

@sonytrinhcvn
Copy link
Collaborator Author

Test 2:

  • Create a new policy
  • My relationship to the policy: The Insurance Agent
  • Create a new case
  • Assign this case to myself

Expect result

  • No email sent - PASSED

@sonytrinhcvn
Copy link
Collaborator Author

Test 3:

  • Create a new policy
  • My relationship to the policy: The Insurance Agent
  • Create a new case
  • Assign this case to someone else The Insurance Company
  • Enter the email address at the Insurance company

Expect result

  • The person assigned to the case receives an email
  • The email includes a link to the case - the link was []
  • The link to the case sends the user to that case in InsureChat

The source code for this email is in the file https://github.com/Unee-T-INS/frontend/blob/master/imports/email-templates/case-assignee-updated.js
The code that is used to generate the URL is

const optOutUrl = createEngagementLink({
url: url.resolve(process.env.ROOT_URL, '/notification-settings'),
id: notificationId,
email: assignee.emails[0].address
})

@sonytrinhcvn
Copy link
Collaborator Author

More Information

In the script, https://github.com/Unee-T-INS/frontend/blob/master/imports/email-templates/components/helpers.js refer to a dependency https://github.com/unee-t/engagement this dependency has not been deployed to Unee-T INS

To do

Deploy unee-t/engagement in the Unee-T INS environment.
Test again

@franck-boullier
Copy link

More information:

Quickfix: remove the engagement URL to avoid relying on the engagement repo.
See unee-t@45c3f63 for the code that'll work without the engagement URLs.

franck-boullier added a commit that referenced this issue Mar 4, 2020
* revert previous hotfix

* replace `property` with `policy`

* do NOT use the function `createEngagementLink` - see GH issue #26
@franck-boullier
Copy link

Quickfix described in #26 (comment) has been implemented as part of PR #27.

We need a long term solution when we:

  • correctly deploy the engagement codebase
  • re-activate the createEngagementLink function so we can better track user engagement on email sent.

To Do:

Test the quick fix.

@franck-boullier
Copy link

The issue has been fixed. See #29 for what we still need to do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants