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.

Sign up for GitHub

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

Subsribe and unsubscribe functionality #3035

Closed
wants to merge 11 commits into from
Closed

Subsribe and unsubscribe functionality #3035

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f438919
Initial commit
BenjaminSsempala Feb 14, 2024
f27d5c8
Adding unsubscription Link
BenjaminSsempala Feb 17, 2024
97c19ab
Fine tuning Subscription Model
BenjaminSsempala Mar 27, 2024
0093a38
Merge branch 'staging' into email-subscribe
BenjaminSsempala Mar 27, 2024
301bc85
Adjusting the schema
BenjaminSsempala Apr 4, 2024
6f4e083
Removing " Product "
BenjaminSsempala Apr 8, 2024
628cccf
Cleanup
BenjaminSsempala Apr 9, 2024
973c8ba
Use email instead of ID
BenjaminSsempala Apr 17, 2024
9473b72
Adding required fields
BenjaminSsempala Apr 23, 2024
6f81753
Update src/auth-service/utils/email.msgs.js
BenjaminSsempala Jun 13, 2024
9c6a76d
Update src/auth-service/config/global/email-templates.js
BenjaminSsempala Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 26 additions & 15 deletions src/auth-service/config/global/email-templates.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
const mongoose = require("mongoose");
const ObjectId = mongoose.Types.ObjectId;

const baseUrl = process.env.PLATFORM_STAGING_BASE_URL || process.env.PLATFORM_PRODUCTION_BASE_URL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @BenjaminSsempala ,

  • Could you share some background notes to this variable declaration? I though environment specific variables are handled under config/environments. Could you shed more light on this?
  • Ideally, the baseURL is supposed to be determined at runtime of the application --- so we might want to just simply call constants.PLATFORM_BASE_URL or constants.ANALYTICS_BASE_URL. See attached screenshot for visual reference.
  • On a side note, we shall also need to be mindful of cyclic dependencies as we go about this.
    Screenshot 2024-04-09 at 07 58 56

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Baalmart , Yes, I failed to use the constants variable because of circular dependencies.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a configuration service for defining URLs.

The use of environment variables directly in the code can lead to hard-coded configurations that are difficult to manage across different environments. Consider using a centralized configuration service or module that can handle these URLs, which can be easily adjusted without changing the codebase.


Comment on lines +4 to +5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a configuration service for defining URLs.

The use of environment variables directly in the code can lead to hard-coded configurations that are difficult to manage across different environments. Consider using a centralized configuration service or module that can handle these URLs, which can be easily adjusted without changing the codebase.

const emailTemplates = {
EMAIL_GREETINGS: function (name) {
return `<tr>
Expand All @@ -23,7 +25,26 @@
</table>
`;
},
EMAIL_FOOTER_TEMPLATE: function (email) {
EMAIL_FOOTER_TEMPLATE: function (email, product, type, paramString) {
let subscriptionBlock = ``;

Check warning on line 29 in src/auth-service/config/global/email-templates.js

View check run for this annotation

Codecov / codecov/patch

src/auth-service/config/global/email-templates.js#L28-L29 

Added lines #L28 - L29 were not covered by tests
if (product && type && paramString) {
const unSubsciptionUrl = `${baseUrl}/api/v2/users/unsubscribe/${product}/${type}?${paramString}`;
subscriptionBlock = `

Check warning on line 32 in src/auth-service/config/global/email-templates.js

View check run for this annotation

Codecov / codecov/patch

src/auth-service/config/global/email-templates.js#L31-L32 

Added lines #L31 - L32 were not covered by tests
<span
style="color: #667085; font-size: 14px; font-family: Inter; font-weight: 400; line-height: 20px; word-wrap: break-word;">.
If you'd rather not receive this kind of email, you can </span>
<a href=${unSubsciptionUrl} target="_blank" <span
style="color: #135DFF; font-size: 14px; font-family: Inter; font-weight: 400; line-height: 20px; word-wrap: break-word;">unsubscribe</span>
</a>
<span
style="color: #667085; font-size: 14px; font-family: Inter; font-weight: 400; line-height: 20px; word-wrap: break-word;">
or </span>
<span
style="color: #135DFF; font-size: 14px; font-family: Inter; font-weight: 400; line-height: 20px; word-wrap: break-word;">manage
your email preferences.</span><br /><br />
`;
}

return `
<table style="width: 100%; text-align: center; padding-top: 32px; padding-bottom: 32px;">
<tr>
Expand Down Expand Up @@ -53,17 +74,7 @@
email was sent to</span>
<span
style="color: #135DFF; font-size: 14px; font-family: Inter; font-weight: 400; line-height: 20px; word-wrap: break-word;">${email}</span>
<span
style="color: #667085; font-size: 14px; font-family: Inter; font-weight: 400; line-height: 20px; word-wrap: break-word;">.
If you'd rather not receive this kind of email, you can </span>
<span
style="color: #135DFF; font-size: 14px; font-family: Inter; font-weight: 400; line-height: 20px; word-wrap: break-word;">unsubscribe</span>
<span
style="color: #667085; font-size: 14px; font-family: Inter; font-weight: 400; line-height: 20px; word-wrap: break-word;">
or </span>
<span
style="color: #135DFF; font-size: 14px; font-family: Inter; font-weight: 400; line-height: 20px; word-wrap: break-word;">manage
your email preferences.</span><br /><br />
${subscriptionBlock}
<span
style="color: #667085; font-size: 14px; font-family: Inter; font-weight: 400; line-height: 20px; word-wrap: break-word;">©
2023 AirQo<br /><br />
Expand All @@ -76,11 +87,11 @@
`;
},

EMAIL_BODY: function (email, content, name) {
const footerTemplate = this.EMAIL_FOOTER_TEMPLATE(email);
EMAIL_BODY: function (email, content, name, product, type, paramString) {
const footerTemplate = this.EMAIL_FOOTER_TEMPLATE(email, product, type, paramString);

Check warning on line 91 in src/auth-service/config/global/email-templates.js

View check run for this annotation

Codecov / codecov/patch

src/auth-service/config/global/email-templates.js#L90-L91 

Added lines #L90 - L91 were not covered by tests
const headerTemplate = this.EMAIL_HEADER_TEMPLATE();
let greetings = this.EMAIL_GREETINGS(name);
if (!name) {
if (!name || name === "") {
greetings = ``;
}
return `<!DOCTYPE html>
Expand Down
16 changes: 11 additions & 5 deletions src/auth-service/models/User.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,17 @@ const UserSchema = new Schema(
category: {
type: String,
},
notifications: {
email: { type: Boolean, default: false },
push: { type: Boolean, default: false },
text: { type: Boolean, default: false },
phone: { type: Boolean, default: false },
mobile_notifications: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a good direction @BenjaminSsempala , love it! :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @BenjaminSsempala ....on a second thought, we might want to rethink this and just maintain one notifications field.......since it already had "phone" in it, we might not need to create a new field for mobile.

Happy to hear your additional thoughts on the same.

email: { type: Boolean, default: true },
push: { type: Boolean, default: true },
text: { type: Boolean, default: true },
phone: { type: Boolean, default: true },
},
analytics_notifications: {
email: { type: Boolean, default: true },
push: { type: Boolean, default: true },
text: { type: Boolean, default: true },
phone: { type: Boolean, default: true },
},
profilePicture: {
type: String,
Expand Down
146 changes: 82 additions & 64 deletions src/auth-service/routes/v2/users.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1041,8 +1041,8 @@ router.get(
);

/*********************************** user notifications **********************/
router.post(
"/subscribe/:type",
router.get(
"/subscribe/:product/:type",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @BenjaminSsempala , thanks for this.

  • Just curious, why are we still adding product as a query parameter in our endpoints?
  • Considering the one account direction, isn't type just enough?

More than happy to dig further into this in a sync of sorts. Thanks again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @Baalmart
The type is for differentiating the category of notification i.e email, push, since each of the platforms has these
The product is for differentiating between the platforms mobile, analytics, web, etc but product describes it better.

Even with the one account, the user would still have multiple platforms available to them e.g how slack has both web push notifications and mobile push notifications.
The alternative would be hardcoding the product and type as one i.e mobile-email, then mobile-push which might be repetitive, especially for future maintenance.

oneOf([
[
query("tenant")
Expand All @@ -1054,40 +1054,49 @@ router.post(
.toLowerCase()
.isIn(["airqo"])
.withMessage("the tenant value is not among the expected ones"),
query("email")
.optional()
.notEmpty()
.withMessage("the email must not be empty if provided")
.bail()
.isEmail()
.withMessage("this is not a valid email address")
.trim(),
query("mongo_user_id")
.optional()
.notEmpty()
.withMessage("the mongo_user_id must not be empty if provided")
.bail()
.trim()
.isMongoId()
.withMessage("the mongo_user_id must be an object ID")
.bail()
.customSanitizer((value) => {
return ObjectId(value);
}),
query("firebase_user_id")
.optional()
.notEmpty()
.withMessage("the firebase_uid must not be empty if provided")
.bail()
.trim(),
],
]),
oneOf([
body("email")
.exists()
.withMessage(
"the user identifier is missing in request, consider using the email"
)
.bail()
.notEmpty()
.withMessage("the email must not be empty if provided")
.bail()
.isEmail()
.withMessage("this is not a valid email address")
.trim(),
body("user_id")
.exists()
.withMessage(
"the user identifier is missing in request, consider using the user_id"
)
.bail()
.notEmpty()
.withMessage("the user_id must not be empty if provided")
.bail()
.trim()
.isMongoId()
.withMessage("the user_id must be an object ID")
.bail()
.customSanitizer((value) => {
return ObjectId(value);
}),
]),
oneOf([
[
param("product")
.exists()
.withMessage("the product must be provided")
.bail()
.notEmpty()
.withMessage("the product should not be empty if provided")
.bail()
.trim()
.toLowerCase()
.isIn(["analytics", "mobile", "website"])
.withMessage(
"the product value is not among the expected ones: analytics, mobile and website"
),
param("type")
.exists()
.withMessage("the type must be provided")
Expand All @@ -1105,8 +1114,8 @@ router.post(
]),
createUserController.subscribeToNotifications
);
router.post(
"/unsubscribe/:type",
router.get(
"/unsubscribe/:product/:type",
oneOf([
[
query("tenant")
Expand All @@ -1118,40 +1127,49 @@ router.post(
.toLowerCase()
.isIn(["airqo"])
.withMessage("the tenant value is not among the expected ones"),
query("email")
.optional()
.notEmpty()
.withMessage("the email must not be empty if provided")
.bail()
.isEmail()
.withMessage("this is not a valid email address")
.trim(),
query("mongo_user_id")
.optional()
.notEmpty()
.withMessage("the mongo_user_id must not be empty if provided")
.bail()
.trim()
.isMongoId()
.withMessage("the mongo_user_id must be an object ID")
.bail()
.customSanitizer((value) => {
return ObjectId(value);
}),
query("firebase_user_id")
.optional()
.notEmpty()
.withMessage("the firebase_uid must not be empty if provided")
.bail()
.trim(),
],
]),
oneOf([
body("email")
.exists()
.withMessage(
"the user identifier is missing in request, consider using the email"
)
.bail()
.notEmpty()
.withMessage("the email must not be empty if provided")
.bail()
.isEmail()
.withMessage("this is not a valid email address")
.trim(),
body("user_id")
.exists()
.withMessage(
"the user identifier is missing in request, consider using the user_id"
)
.bail()
.notEmpty()
.withMessage("the user_id must not be empty if provided")
.bail()
.trim()
.isMongoId()
.withMessage("the user_id must be an object ID")
.bail()
.customSanitizer((value) => {
return ObjectId(value);
}),
]),
oneOf([
[
param("product")
.exists()
.withMessage("the product must be provided")
.bail()
.notEmpty()
.withMessage("the product should not be empty if provided")
.bail()
.trim()
.toLowerCase()
.isIn(["analytics", "mobile", "website"])
.withMessage(
"the product value is not among the expected ones: analytics, mobile and website"
),
param("type")
.exists()
.withMessage("the type must be provided")
Expand Down
4 changes: 4 additions & 0 deletions src/auth-service/utils/control-access.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ const isIPBlacklistedHelper = async (
token = "",
name = "",
client_id = "",
user_id = "",
} = (accessToken && accessToken._doc) || {};

const BLOCKED_IP_PREFIXES =
Expand Down Expand Up @@ -390,6 +391,7 @@ const isIPBlacklistedHelper = async (
firstName,
lastName,
ip,
user_id
},
next
);
Expand Down Expand Up @@ -579,6 +581,7 @@ const controlAccess = {
firstName: userDetails[0].firstName,
username: userDetails[0].userName,
email: userDetails[0].email,
user_id: userDetails[0]._id,
},
next
);
Expand Down Expand Up @@ -1057,6 +1060,7 @@ const controlAccess = {
username: user.userName,
password,
email: user.email,
user_id: user._id,
},
next
);
Expand Down
3 changes: 3 additions & 0 deletions src/auth-service/utils/create-candidate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const createCandidate = {
email,
firstName,
lastName,
user_id: userExists._id,
},
next
);
Expand All @@ -86,12 +87,14 @@ const createCandidate = {

if (responseFromCreateCandidate.success === true) {
const createdCandidate = await responseFromCreateCandidate.data;
const user_id = createdCandidate._id;
const responseFromSendEmail = await mailer.candidate(
{
firstName,
lastName,
email,
tenant,
user_id
},
next
);
Expand Down
5 changes: 5 additions & 0 deletions src/auth-service/utils/create-inquiry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
const UserModel = require("@models/User");
const InquiryModel = require("@models/Inquiry");
const { logObject } = require("@utils/log");
const mailer = require("@utils/mailer");
const CreateUserUtil = require("@utils/create-user");
const httpStatus = require("http-status");
const constants = require("@config/constants");
const generatFilter = require("@utils/generate-filter");
Expand Down Expand Up @@ -32,6 +34,8 @@ const inquiry = {
);

if (responseFromCreateInquiry.success === true) {
const responseFromListUser = await CreateUserUtil.list(request, next);
const user_id = responseFromListUser.data[0]._id;
const createdInquiry = await responseFromCreateInquiry.data;
const responseFromSendEmail = await mailer.inquiry(
{
Expand All @@ -40,6 +44,7 @@ const inquiry = {
category,
message,
tenant,
user_id
},
next
);
Expand Down
3 changes: 3 additions & 0 deletions src/auth-service/utils/create-request.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ const createAccessRequest = {
email: user.email,
tenant,
entity_title: group.grp_title,
user_id: user._id,
},
next
);
Expand Down Expand Up @@ -383,6 +384,7 @@ const createAccessRequest = {
username: email,
email,
entity_title,
user_id: newUser._id,
},
next
);
Expand Down Expand Up @@ -486,6 +488,7 @@ const createAccessRequest = {
email: user.email,
tenant,
entity_title: network.net_name,
user_id: user._id,
},
next
);
Expand Down
Loading
Loading