-
Notifications
You must be signed in to change notification settings - Fork 3
user invite #244
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
base: main
Are you sure you want to change the base?
user invite #244
Conversation
Reviewer's GuideThis PR implements a complete user invitation flow by extending GraphQL mutations, integrating a new EmailDeliveries model, introducing reusable email utilities and templates, and updating related schemas and filters to support transactional email delivery and ID-based filtering. Sequence diagram for the user invitation flowsequenceDiagram
actor Admin
participant API as GraphQL API
participant UsersModel as Users Model
participant EmailUtils as Email Utils
participant EmailDeliveries as EmailDeliveries Model
Admin->>API: inviteUsers mutation
API->>UsersModel: checkDuplication(email)
UsersModel-->>API: result
API->>UsersModel: invite(doc)
UsersModel-->>API: token
API->>UsersModel: findOne(email)
UsersModel-->>API: createdUser
API->>EmailUtils: sendInvitationEmail(models, subdomain, { email, token })
EmailUtils->>EmailDeliveries: createEmailDelivery(doc)
EmailDeliveries-->>EmailUtils: EmailDeliveryDocument
EmailUtils->>EmailUtils: sendEmail (via transporter or sendgrid)
EmailUtils-->>API: (email sent)
API->>UsersModel: (optional) updateUserChannels
API->>EmailUtils: resetPermissionsCache(models)
API-->>Admin: (invitation sent)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThis update introduces a comprehensive email delivery system to the backend, including new Mongoose models, schemas, and utility functions for email sending, logging, and templating. It also adds multiple HTML email templates for various scenarios, enhances GraphQL and filtering logic for contacts, and improves user invitation validation and workflow. The package now includes the Handlebars templating library. Changes
Sequence Diagram(s)sequenceDiagram
participant API
participant User
participant Models
participant EmailUtils
participant MailTransport
participant DB
User->>API: Request user invitation
API->>Models: Validate groupId
Models-->>API: GroupId valid/invalid
API->>Models: Create invited user
Models-->>API: User created
API->>EmailUtils: sendInvitationEmail(email, token)
EmailUtils->>MailTransport: Prepare and send email (template, data)
MailTransport-->>EmailUtils: Email sent
EmailUtils->>DB: Log email delivery
DB-->>EmailUtils: Delivery logged
API->>Models: Reset permissions cache
API-->>User: Respond with invitation result
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @batmnkh2344 - I've reviewed your changes - here's some feedback:
- The
utils.tsemail module is very large and mixes template loading, transporter setup, and sending logic; consider splitting it into smaller, single-responsibility modules or services to improve readability and maintainability. - The
userMutationsinvite flow awaits every step sequentially (duplication check, invite, branch update, messages, email, cache reset), which may be slow for bulk invites—consider parallelizing independent operations where safe. - You changed
excludeIdsfrom a Boolean to a String array in the GraphQL schema and filter util—ensure that all resolvers and front-end consumers are updated to handle the new list type to avoid regressions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `utils.ts` email module is very large and mixes template loading, transporter setup, and sending logic; consider splitting it into smaller, single-responsibility modules or services to improve readability and maintainability.
- The `userMutations` invite flow awaits every step sequentially (duplication check, invite, branch update, messages, email, cache reset), which may be slow for bulk invites—consider parallelizing independent operations where safe.
- You changed `excludeIds` from a Boolean to a String array in the GraphQL schema and filter util—ensure that all resolvers and front-end consumers are updated to handle the new list type to avoid regressions.
## Individual Comments
### Comment 1
<location> `backend/core-api/src/modules/organization/team-member/db/definitions/emailDeliveries.ts:10` </location>
<code_context>
+ subject: { type: String },
+ body: { type: String },
+ to: { type: [String] },
+ cc: { type: [String], optional: true },
+ bcc: { type: [String], optional: true },
+ attachments: { type: [Object] },
</code_context>
<issue_to_address>
The 'optional' property is not a standard Mongoose schema option.
Remove 'optional: true' from the schema fields, as Mongoose ignores this property. Fields are optional by default unless 'required: true' is specified.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
cc: { type: [String], optional: true },
bcc: { type: [String], optional: true },
=======
cc: { type: [String] },
bcc: { type: [String] },
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `backend/core-api/src/modules/organization/team-member/db/definitions/emailDeliveries.ts:18` </location>
<code_context>
+ customerId: { type: String },
+ userId: { type: String },
+ createdAt: { type: Date, default: Date.now },
+ status: { type: String, enum: EMAIL_DELIVERY_STATUS.ALL },
+});
</code_context>
<issue_to_address>
Consider setting a default value for 'status'.
Setting a default value like 'pending' for 'status' can help prevent missing or inconsistent values.
Suggested implementation:
```typescript
status: { type: String, enum: EMAIL_DELIVERY_STATUS.ALL, default: 'pending' },
```
If `'pending'` is a value in `EMAIL_DELIVERY_STATUS`, consider using `EMAIL_DELIVERY_STATUS.PENDING` instead of the string literal `'pending'` for better maintainability:
```ts
default: EMAIL_DELIVERY_STATUS.PENDING
```
</issue_to_address>
### Comment 3
<location> `backend/core-api/src/modules/organization/team-member/graphql/mutations.ts:248` </location>
<code_context>
+ });
+ }
+
+ sendInvitationEmail(models, subdomain, { email: entry.email, token });
}
+
</code_context>
<issue_to_address>
sendInvitationEmail is called without awaiting its result.
If you need to handle errors or ensure the email is sent before continuing, use 'await'. If fire-and-forget is intentional, please document this choice.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| cc: { type: [String], optional: true }, | ||
| bcc: { type: [String], optional: true }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: The 'optional' property is not a standard Mongoose schema option.
Remove 'optional: true' from the schema fields, as Mongoose ignores this property. Fields are optional by default unless 'required: true' is specified.
| cc: { type: [String], optional: true }, | |
| bcc: { type: [String], optional: true }, | |
| cc: { type: [String] }, | |
| bcc: { type: [String] }, |
| customerId: { type: String }, | ||
| userId: { type: String }, | ||
| createdAt: { type: Date, default: Date.now }, | ||
| status: { type: String, enum: EMAIL_DELIVERY_STATUS.ALL }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Consider setting a default value for 'status'.
Setting a default value like 'pending' for 'status' can help prevent missing or inconsistent values.
Suggested implementation:
status: { type: String, enum: EMAIL_DELIVERY_STATUS.ALL, default: 'pending' },If 'pending' is a value in EMAIL_DELIVERY_STATUS, consider using EMAIL_DELIVERY_STATUS.PENDING instead of the string literal 'pending' for better maintainability:
default: EMAIL_DELIVERY_STATUS.PENDING| }); | ||
| } | ||
|
|
||
| sendInvitationEmail(models, subdomain, { email: entry.email, token }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question (bug_risk): sendInvitationEmail is called without awaiting its result.
If you need to handle errors or ensure the email is sent before continuing, use 'await'. If fire-and-forget is intentional, please document this choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Changes requested ❌
Reviewed everything up to c11e3bb in 2 minutes and 19 seconds. Click for details.
- Reviewed
2397lines of code in20files - Skipped
0files when reviewing. - Skipped posting
10draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. backend/core-api/src/modules/contacts/graphql/schemas/customer.ts:77
- Draft comment:
Changed the type of 'excludeIds' from Boolean to [String]. Ensure that downstream resolvers and the front‐end correctly handle an array rather than a Boolean. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to ensure that downstream resolvers and the front-end handle the change correctly. This falls under asking the author to ensure behavior is intended or tested, which is against the rules.
2. backend/core-api/src/modules/contacts/utils.ts:51
- Draft comment:
Multiple sequential calls to findIntegrations (for brandIds, integrationIds, integrationTypes) may impact performance. Consider running these queries concurrently (e.g. using Promise.all). - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. backend/core-api/src/modules/organization/team-member/db/models/Users.ts:754
- Draft comment:
User lookup in the login function constructs a RegExp using raw user input. To mitigate potential regex injection issues, escape special characters (e.g. using an escape utility) before building the RegExp. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. backend/core-api/src/modules/organization/team-member/db/models/Users.ts:516
- Draft comment:
The generatePassword function first applies a SHA256 hash and then bcrypt. Consider using bcrypt directly on the raw password unless prehashing is intentional for normalization, as double hashing may reduce the benefits of bcrypt's salting. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. backend/core-api/src/modules/organization/team-member/db/models/Users.ts:867
- Draft comment:
In the findUsers method, there is a try-catch block that calls models.Users.find(), but its result isn’t used inside the catch block. This redundancy could be refactored to properly handle any errors. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. backend/core-api/src/modules/organization/team-member/utils.ts:222
- Draft comment:
In the sendEmail function, the emailDelivery document is cast to 'any'. For better type safety and maintainability, consider defining and using proper types for the email delivery document. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While using proper types is generally good practice, in this case: 1) We don't have access to the EmailDeliveries model definition 2) The object is only used to get its _id property 3) This is a new file being added, so the current code works as intended 4) The type casting doesn't pose any real risk since the usage is very limited The comment does identify a legitimate TypeScript best practice issue. Proper typing could help catch errors if the EmailDeliveries API changes. However, the current usage is very simple and contained. The risk of issues is minimal, and fixing this would require access to types we don't have in this PR's context. While technically correct, this comment is not important enough to keep given the limited scope of the emailDelivery usage and lack of access to the full type definitions.
7. package.json:3
- Draft comment:
The 'version' field is set to '0.0.0' in package.json, which is inconsistent with the version specified in private/version.json ('0.17.6'). Consider synchronizing these versions. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. backend/core-api/src/modules/organization/team-member/db/models/Users.ts:8
- Draft comment:
Typo detected: 'userMovemmentSchema' appears to be a misspelling. Consider changing it to 'userMovementSchema'. - Reason this comment was not posted:
Comment was on unchanged code.
9. backend/core-api/src/private/emailTemplates/notification.html:39
- Draft comment:
Typo alert: The header's background color is defined as #5629b6, but the border-left in the notification content uses #5628b6. Please confirm if this is intentional or if it should be consistent. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a UI styling issue with an extremely minor color difference (one hex digit). The difference is likely unnoticeable to the human eye. According to the rules, we should NOT comment on pure frontend UI or styling changes - we should assume the author made these choices intentionally. This seems like an unnecessary nitpick that doesn't affect functionality. The color inconsistency could potentially be a copy-paste error that the author would want to fix for code consistency. Maybe there's a design system that specifies exact colors. Even if it was a copy-paste error, the difference is so minor that it doesn't impact the user experience. The rules explicitly state not to comment on UI/styling changes. Delete this comment as it relates to pure UI styling which we should assume was intentional according to the rules.
10. backend/core-api/src/private/emailTemplates/userInvitation.html:122
- Draft comment:
Typographical suggestion: The phrase "become team member" might read better as "become a team member". - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a new email template file being added. The grammar suggestion is technically correct - it would read better with "a". However, the rules state not to make purely informative comments or unimportant suggestions. While this is a customer-facing email template, the grammar issue is very minor and doesn't impact understanding. The text will be seen by users in emails, so grammar and professionalism could be considered important. Poor grammar might reflect badly on the company. While grammar is important, this is such a minor issue that it doesn't warrant a PR comment. The meaning is perfectly clear either way. Delete this comment as it's too minor of a suggestion and falls under the "do not make purely informative comments" rule.
Workflow ID: wflow_0HuaQ6C6KxsGj6sG
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
| subject: { type: String }, | ||
| body: { type: String }, | ||
| to: { type: [String] }, | ||
| cc: { type: [String], optional: true }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The schema for 'cc' (and similarly 'bcc') uses 'optional: true'. In Mongoose the standard option is 'required: false'. Verify that the intended optional behavior is achieved.
| cc: { type: [String], optional: true }, | |
| cc: { type: [String], required: false }, |
| } | ||
| } catch (e) { | ||
| // return debugError(e.message); | ||
| console.log(e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the sendEmail function, if transporter creation fails, the error is caught and only logged (via console.log) without proper error handling. Consider using a structured logging mechanism or rethrowing the error to ensure issues are visible.
| padding: 0; | ||
| -webkit-text-size-adjust: 100%; | ||
| -ms-text-size-adjust: 100%; | ||
| font-family: 'Roboto', 'Helvetica Neue', 'Helvetica', 'Arial' sans-serif; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typographical error: In the font-family definition, it appears there's a missing comma between 'Arial' and sans-serif. It should likely be: font-family: 'Roboto', 'Helvetica Neue', 'Helvetica', 'Arial', sans-serif;
| font-family: 'Roboto', 'Helvetica Neue', 'Helvetica', 'Arial' sans-serif; | |
| font-family: 'Roboto', 'Helvetica Neue', 'Helvetica', 'Arial', sans-serif; |
| padding: 0; | ||
| -webkit-text-size-adjust: 100%; | ||
| -ms-text-size-adjust: 100%; | ||
| font-family: 'Roboto', 'Helvetica Neue', 'Helvetica', 'Arial' sans-serif; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: In the font-family declaration, consider adding a comma between 'Arial' and sans-serif (i.e., it should be 'Arial', sans-serif) for proper syntax.
| font-family: 'Roboto', 'Helvetica Neue', 'Helvetica', 'Arial' sans-serif; | |
| font-family: 'Roboto', 'Helvetica Neue', 'Helvetica', 'Arial', sans-serif; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 18
🔭 Outside diff range comments (1)
backend/core-api/src/private/emailTemplates/notification.html (1)
181-184: Remove JavaScript as it won't execute in most email clients.The JavaScript code to set the current year will not work in most email clients, as they typically strip or disable JavaScript for security reasons. Consider either hardcoding the year or handling this server-side when the template is rendered.
-<script> - document.getElementById("year").innerHTML = new Date().getFullYear(); -</script> +<!-- Year should be populated server-side when rendering the template -->And update the copyright line to use a server-side placeholder:
- Copyright © <span id="year"></span> + Copyright © {{ currentYear }}
🧹 Nitpick comments (15)
backend/core-api/src/modules/contacts/utils.ts (1)
16-18: Consider improving type safety for the params parameter.While the destructuring logic is correct, the function uses
anyfor theparamsparameter type. Consider defining a proper interface to improve type safety.+interface FilterParams { + searchValue?: string; + tagIds?: string[]; + excludeTagIds?: string[]; + tagWithRelated?: boolean; + type?: string; + dateFilters?: string; + brandIds?: string[]; + integrationIds?: string[]; + integrationTypes?: string[]; + status?: string; + ids?: string[]; + excludeIds?: string[]; +} -export const generateFilter = async (params: any, models: IModels) => { +export const generateFilter = async (params: FilterParams, models: IModels) => {backend/core-api/src/private/emailTemplates/invitation.html (2)
1-1: Add DOCTYPE declaration for valid HTML.The static analysis correctly identifies that a DOCTYPE declaration is missing.
+<!DOCTYPE html> +<html> +<head> + <meta charset="utf-8"> + <meta name="viewport" content="width=device-width, initial-scale=1.0"> +</head> +<body> <table role="presentation" cellpadding="0"Don't forget to close the HTML tags at the end of the file:
<script> document.getElementById("year").innerHTML = new Date().getFullYear(); </script> +</body> +</html>
16-28: Consider hosting images locally or using data URIs.External image dependencies from imgur.com could break if the URLs become unavailable, affecting email presentation.
Consider hosting these images in your application's static assets or using data URIs for critical branding elements to ensure reliable email rendering.
backend/core-api/src/private/emailTemplates/unsubscribe.html (1)
4-4: Add a meaningful title for better email client compatibility.The empty
<title>tag should contain descriptive text to improve accessibility and email client compatibility.- <title></title> + <title>Unsubscribe Successful - erxes</title>backend/core-api/src/private/emailTemplates/base.html (1)
8-8: Add a default title for the base template.The empty
<title>tag should have a default value that can be overridden by specific templates.- <title></title> + <title>erxes</title>backend/core-api/src/private/emailTemplates/conversationCron.html (1)
5-5: Add a meaningful title for the conversation email.The empty
<title>tag should contain descriptive text for better email client compatibility.- <title></title> + <title>Conversation Update - erxes</title>backend/core-api/src/private/emailTemplates/notification.html (1)
1-8: Consider adding DOCTYPE declaration for better email client compatibility.While email templates often omit DOCTYPE declarations, adding one can improve rendering consistency across email clients.
Consider adding this DOCTYPE at the beginning:
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> <table role="presentation"backend/core-api/src/modules/organization/team-member/db/models/EmailDeliveries.ts (2)
14-33: Consider using plain functions instead of static-only class.The static analysis tool correctly identifies that this class contains only static members. While this pattern works, it could be simplified.
Consider refactoring to use plain functions instead:
-export const loadEmailDeliveryClass = (models: IModels) => { - class EmailDelivery { - /** - * Create an EmailDelivery document - */ - public static async createEmailDelivery(doc: IEmailDeliveries) { - return models.EmailDeliveries.create({ - ...doc, - }); - } - - public static async updateEmailDeliveryStatus(_id: string, status: string) { - return models.EmailDeliveries.updateOne({ _id }, { $set: { status } }); - } - } - - emailDeliveriesSchema.loadClass(EmailDelivery); - - return emailDeliveriesSchema; -}; +export const loadEmailDeliveryClass = (models: IModels) => { + const createEmailDelivery = async (doc: IEmailDeliveries) => { + return models.EmailDeliveries.create({ + ...doc, + }); + }; + + const updateEmailDeliveryStatus = async (_id: string, status: string) => { + return models.EmailDeliveries.updateOne({ _id }, { $set: { status } }); + }; + + emailDeliveriesSchema.statics.createEmailDelivery = createEmailDelivery; + emailDeliveriesSchema.statics.updateEmailDeliveryStatus = updateEmailDeliveryStatus; + + return emailDeliveriesSchema; +};However, the current implementation is also acceptable and follows established Mongoose patterns in the codebase.
25-27: Consider returning the updated document for better API consistency.The
updateEmailDeliveryStatusmethod currently returns the result ofupdateOne, but it might be more useful to return the updated document for consistency with other model methods.- public static async updateEmailDeliveryStatus(_id: string, status: string) { - return models.EmailDeliveries.updateOne({ _id }, { $set: { status } }); - } + public static async updateEmailDeliveryStatus(_id: string, status: string) { + return models.EmailDeliveries.findOneAndUpdate( + { _id }, + { $set: { status } }, + { new: true } + ); + }backend/core-api/src/private/emailTemplates/magicLogin.html (2)
1-16: Add DOCTYPE declaration for consistency.Same recommendation as for notification.html - consider adding DOCTYPE for better email client compatibility.
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> <table
48-70: Verify external image dependencies.The template relies on external imgur.com URLs which could become unavailable. Consider hosting these images on your own infrastructure.
backend/core-api/src/private/emailTemplates/resetPassword.html (1)
17-30: Consider hosting images internally for reliability and securityThe template relies on external image hosting (imgur.com), which introduces potential points of failure and security concerns.
Consider:
- Hosting images on your own CDN or server
- Using base64 encoded images for critical assets
- Implementing fallback text for when images fail to load
Also applies to: 99-159
backend/core-api/src/modules/organization/team-member/utils.ts (1)
234-234: Use optional chaining for cleaner codeApply this improvement:
- EmailDeliveryId: emailDelivery && emailDelivery._id, + EmailDeliveryId: emailDelivery?._id,backend/core-api/src/modules/organization/team-member/types.ts (2)
3-8: Consider adding validation constraints to IAttachmentParams.The interface could benefit from more specific type constraints to improve type safety and validation.
export interface IAttachmentParams { - data: string; + data: string; // Base64 encoded data filename: string; - size: number; + size: number; // Size in bytes, should be positive - mimeType: string; + mimeType: string; // MIME type like 'image/jpeg', 'application/pdf', etc. }Additionally, consider adding runtime validation or using a more specific type for
mimeType:export type SupportedMimeType = 'image/jpeg' | 'image/png' | 'application/pdf' | 'text/plain' | string;
28-39: Consider improving type safety for IEmailParams.Some fields could benefit from more specific typing to improve type safety.
export interface IEmailParams { toEmails?: string[]; fromEmail?: string; title?: string; customHtml?: string; customHtmlData?: any; - template?: { name?: string; data?: any }; + template?: { + name: string; // Make name required if template is provided + data?: Record<string, any>; // More specific than 'any' + }; - attachments?: object[]; + attachments?: IAttachmentParams[]; modifier?: (data: any, email: string) => void; transportMethod?: string; - getOrganizationDetail?: ({ subdomain }: { subdomain: string }) => any; + getOrganizationDetail?: ({ subdomain }: { subdomain: string }) => Promise<any>; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
backend/core-api/src/connectionResolvers.ts(3 hunks)backend/core-api/src/modules/contacts/graphql/schemas/customer.ts(1 hunks)backend/core-api/src/modules/contacts/utils.ts(2 hunks)backend/core-api/src/modules/organization/team-member/constants.ts(1 hunks)backend/core-api/src/modules/organization/team-member/db/definitions/emailDeliveries.ts(1 hunks)backend/core-api/src/modules/organization/team-member/db/models/EmailDeliveries.ts(1 hunks)backend/core-api/src/modules/organization/team-member/db/models/Users.ts(2 hunks)backend/core-api/src/modules/organization/team-member/graphql/mutations.ts(4 hunks)backend/core-api/src/modules/organization/team-member/types.ts(1 hunks)backend/core-api/src/modules/organization/team-member/utils.ts(1 hunks)backend/core-api/src/private/emailTemplates/base.html(1 hunks)backend/core-api/src/private/emailTemplates/conversationCron.html(1 hunks)backend/core-api/src/private/emailTemplates/invitation.html(1 hunks)backend/core-api/src/private/emailTemplates/magicLogin.html(1 hunks)backend/core-api/src/private/emailTemplates/notification.html(1 hunks)backend/core-api/src/private/emailTemplates/resetPassword.html(1 hunks)backend/core-api/src/private/emailTemplates/unsubscribe.html(1 hunks)backend/core-api/src/private/emailTemplates/userInvitation.html(1 hunks)backend/core-api/src/private/version.json(1 hunks)package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.ts`: Use TypeScript for all code; prefer interfaces over types. Avoid enums; use maps instead.
**/*.ts: Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use maps instead.
📄 Source: CodeRabbit Inference Engine (.cursorrules)
List of files the instruction was applied to:
backend/core-api/src/modules/contacts/utils.tsbackend/core-api/src/modules/organization/team-member/constants.tsbackend/core-api/src/modules/organization/team-member/db/models/EmailDeliveries.tsbackend/core-api/src/modules/contacts/graphql/schemas/customer.tsbackend/core-api/src/modules/organization/team-member/db/definitions/emailDeliveries.tsbackend/core-api/src/modules/organization/team-member/db/models/Users.tsbackend/core-api/src/connectionResolvers.tsbackend/core-api/src/modules/organization/team-member/graphql/mutations.tsbackend/core-api/src/modules/organization/team-member/types.tsbackend/core-api/src/modules/organization/team-member/utils.ts
🧬 Code Graph Analysis (3)
backend/core-api/src/modules/organization/team-member/db/definitions/emailDeliveries.ts (2)
backend/erxes-api-shared/src/utils/mongo/mongoose-types.ts (1)
mongooseStringRandomId(4-7)backend/core-api/src/modules/organization/team-member/constants.ts (1)
EMAIL_DELIVERY_STATUS(1-5)
backend/core-api/src/connectionResolvers.ts (2)
backend/core-api/src/modules/organization/team-member/db/models/EmailDeliveries.ts (2)
IEmailDeliveryModel(9-12)loadEmailDeliveryClass(14-33)backend/core-api/src/modules/organization/team-member/types.ts (1)
IEmailDeliveriesDocument(24-26)
backend/core-api/src/modules/organization/team-member/graphql/mutations.ts (6)
backend/core-api/src/connectionResolvers.ts (1)
IContext(188-192)backend/gateway/src/connectionResolver.ts (1)
IContext(20-23)backend/erxes-api-shared/src/utils/service-discovery.ts (1)
isEnabled(82-88)backend/erxes-api-shared/src/utils/trpc/index.ts (1)
sendTRPCMessage(42-68)backend/core-api/src/modules/organization/team-member/utils.ts (1)
sendInvitationEmail(263-292)backend/core-api/src/modules/permissions/utils.ts (1)
resetPermissionsCache(11-19)
🪛 HTMLHint (1.5.0)
backend/core-api/src/private/emailTemplates/base.html
[error] 8-8: <title></title> must not be empty.
(title-require)
backend/core-api/src/private/emailTemplates/invitation.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
backend/core-api/src/private/emailTemplates/unsubscribe.html
[error] 4-4: <title></title> must not be empty.
(title-require)
backend/core-api/src/private/emailTemplates/userInvitation.html
[error] 72-72: <title> must be present in tag.
(title-require)
[error] 144-144: Duplicate of attribute name [ target ] was found.
(attr-no-duplication)
backend/core-api/src/private/emailTemplates/conversationCron.html
[error] 5-5: <title></title> must not be empty.
(title-require)
[error] 104-104: Tag must be paired, missing: [ ], start tag match failed [ ] on line 104.
(tag-pair)
[error] 158-158: Tag must be paired, no start tag: [ ]
(tag-pair)
[error] 159-159: Tag must be paired, no start tag: [ ]
(tag-pair)
[error] 160-160: Tag must be paired, no start tag: [ ]
(tag-pair)
[error] 161-161: Tag must be paired, no start tag: [ ]
(tag-pair)
[error] 187-187: The value of attribute [ src ] must be in double quotes.
(attr-value-double-quotes)
[error] 237-237: The value of attribute [ src ] must be in double quotes.
(attr-value-double-quotes)
[error] 255-255: Tag must be paired, no start tag: [ ]
(tag-pair)
backend/core-api/src/private/emailTemplates/notification.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
backend/core-api/src/private/emailTemplates/resetPassword.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
backend/core-api/src/private/emailTemplates/magicLogin.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🪛 Biome (1.9.4)
backend/core-api/src/modules/organization/team-member/db/models/EmailDeliveries.ts
[error] 15-28: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
backend/core-api/src/modules/organization/team-member/utils.ts
[error] 234-234: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (35)
backend/core-api/src/modules/contacts/graphql/schemas/customer.ts (1)
78-78: LGTM! Type change improves API flexibility.The change from Boolean to
[String]forexcludeIdsenables excluding multiple customer IDs in GraphQL queries, which is a significant improvement over the previous boolean-only approach.backend/core-api/src/modules/contacts/utils.ts (1)
35-46: LGTM! ID filtering logic is well-implemented.The conditional logic correctly handles all three scenarios for ID-based filtering:
- Both inclusion and exclusion using
$inand$nin- Inclusion only using
$in- Exclusion only using
$ninThe use of optional chaining (
?.length) ensures safe array checking, and the MongoDB query structure is correct.package.json (1)
99-99: Verify handlebars version for security and currency.Please confirm that handlebars ^4.7.8 is the latest secure version without known vulnerabilities.
What is the latest version of handlebars and are there any known security vulnerabilities in handlebars 4.7.8?backend/core-api/src/modules/organization/team-member/constants.ts (1)
1-5: Well-structured constant definition.The EMAIL_DELIVERY_STATUS constant is cleanly defined with appropriate string values and an ALL array for validation purposes. This follows TypeScript best practices.
backend/core-api/src/modules/organization/team-member/db/models/Users.ts (1)
325-327: Excellent validation enhancement.Adding group ID validation prevents invitations with invalid groups, improving data integrity and user experience by catching invalid references early.
backend/core-api/src/modules/organization/team-member/db/definitions/emailDeliveries.ts (1)
5-19: Well-designed email delivery schema.The schema structure is comprehensive with appropriate field types and proper use of enum validation for the status field.
backend/core-api/src/private/emailTemplates/invitation.html (1)
38-42: Template structure is well-designed for email clients.The use of handlebars placeholders and table-based layout with inline CSS follows email template best practices for cross-client compatibility.
backend/core-api/src/private/emailTemplates/unsubscribe.html (1)
20-36: Well-structured email template with proper inline styling.The template uses appropriate inline CSS and responsive design patterns for email clients. The centered layout and branding elements are well-implemented.
backend/core-api/src/private/emailTemplates/userInvitation.html (1)
47-72: Good email client compatibility implementation.The template includes comprehensive CSS rules for Outlook compatibility and responsive design, which is essential for email templates.
backend/core-api/src/private/emailTemplates/base.html (1)
83-105: Excellent base template structure for email delivery system.The template provides a solid foundation with proper email client compatibility features and a flexible content placeholder using Handlebars syntax.
backend/core-api/src/connectionResolvers.ts (3)
133-141: Clean integration of email delivery model imports.The imports are properly organized and follow the existing import patterns in the file.
184-184: Correct interface extension for new model.The
EmailDeliveriesproperty is properly added to theIModelsinterface with the correct type.
335-338: Proper model registration following established patterns.The email delivery model registration follows the same pattern as other models in the system, using the correct document interface and model interface types.
backend/core-api/src/private/emailTemplates/conversationCron.html (1)
181-244: Good use of Handlebars templating for dynamic content.The template effectively uses Handlebars syntax for displaying conversation data, including the
{{#each answers}}loop for rendering multiple responses.backend/core-api/src/private/emailTemplates/notification.html (3)
17-31: Verify external image URL reliability.The template uses hardcoded external image URLs from imgur.com. If these services become unavailable, the email appearance will be degraded.
Consider hosting these images on your own CDN or verifying that these URLs are stable long-term assets.
#!/bin/bash # Verify image URLs are accessible urls=( "https://i.imgur.com/Obpdd9r.png" "https://i.imgur.com/7pXtCWF.png" ) for url in "${urls[@]}"; do echo "Checking $url" curl -I "$url" -o /dev/null -s -w "%{http_code}\n" done
1-8: Missing DOCTYPE is acceptable for email templates.While the static analysis tool flags the missing DOCTYPE, this is common and often intentional in email templates since many email clients strip or ignore DOCTYPE declarations. The current structure is appropriate for email compatibility.
38-41: ```shell
#!/bin/bashShow context around template loading and compilation
sed -n '1,120p' backend/core-api/src/modules/organization/team-member/utils.ts
echo "----"
sed -n '170,240p' backend/core-api/src/modules/organization/team-member/utils.ts</details> <details> <summary>backend/core-api/src/modules/organization/team-member/db/models/EmailDeliveries.ts (2)</summary> `9-12`: **Well-defined interface structure.** The interface correctly extends Mongoose's Model with appropriate static method signatures. --- `15-28`: **Static-only class is acceptable in this Mongoose context.** While the static analysis tool suggests avoiding classes with only static members, this pattern is standard for Mongoose models where static methods are added via `loadClass`. The current implementation is consistent with Mongoose best practices and likely matches other models in the codebase. </details> <details> <summary>backend/core-api/src/modules/organization/team-member/graphql/mutations.ts (6)</summary> `7-10`: **Well-organized imports for new functionality.** The import additions properly support the new email sending and caching features. --- `204-204`: **Appropriate context parameter extraction.** Adding `subdomain` to the destructured context parameters is necessary for the email sending functionality. --- `238-249`: **Excellent conditional service integration.** The code properly checks if the 'frontline' service is enabled before attempting to send TRPC messages, preventing errors in environments where the service isn't available. The error handling and service availability check demonstrate good defensive programming practices. --- `248-248`: **Proper email sending integration.** The invitation email sending is correctly integrated into the user creation workflow with appropriate parameters. --- `251-251`: **Strategic cache reset placement.** Resetting the permissions cache after processing all invitation entries is efficient and ensures consistency. --- `204-204`: To ensure the `subdomain` field is actually part of your GraphQL context interface, let’s locate the `IContext` declaration(s) and verify whether `subdomain` is defined: ```shell #!/bin/bash # Locate IContext declarations (interface or type) and check for subdomain declare -a patterns=("interface IContext" "type IContext") found=false for pat in "${patterns[@]}"; do files=$(rg -l "$pat" --type ts) if [[ -n "$files" ]]; then found=true for file in $files; do echo "---- $file ----" # Print the start of the declaration sed -n '1,200p' "$file" echo "-- Checking for 'subdomain' property --" grep -n "subdomain" "$file" || echo " → subdomain not found in this declaration" echo done fi done if ! $found; then echo "No IContext declarations (interface or type) were found in .ts files." fibackend/core-api/src/modules/organization/team-member/types.ts (5)
3-8: Well-defined attachment interface.The
IAttachmentParamsinterface covers all necessary properties for email attachments with appropriate types.
10-22: Comprehensive email delivery interface.The
IEmailDeliveriesinterface properly models email delivery records with all necessary fields, including optional properties where appropriate.
24-26: Correct document interface extension.The document interface properly extends both the domain interface and Mongoose's Document interface.
28-39: Flexible email parameters interface.The
IEmailParamsinterface provides good flexibility for various email sending scenarios with appropriate optional fields.
10-22: LGTM! Well-structured email delivery interface.The
IEmailDeliveriesinterface is well-designed with appropriate optional fields and covers all necessary email delivery attributes.backend/core-api/src/private/emailTemplates/magicLogin.html (4)
110-127: Secure magic login implementation.The template properly uses triple braces
{{{loginUrl}}}for unescaped HTML, which is appropriate for URLs. The security messaging and 10-minute expiration notice are good practices.
84-86: Consider security implications of 10-minute expiration.The template mentions a 10-minute expiration for the login link. Ensure this timeframe aligns with your security policy and that the server-side implementation properly enforces this expiration.
1-16: Email template structure is well-designed.The table-based layout with inline CSS is appropriate for email client compatibility. Missing DOCTYPE is acceptable for email templates.
112-112: ```shell
#!/bin/bashBroad search for where loginUrl is set or passed into templates
rg -n "loginUrl" .
Search for any code referencing the magic login template or function
rg -n "magicLogin" .
</details> <details> <summary>backend/core-api/src/private/emailTemplates/resetPassword.html (1)</summary> `181-183`: ```shell #!/bin/bash set -e echo "---- Searching for '{{' in resetPassword.html ----" rg -n "\{\{" backend/core-api/src/private/emailTemplates/resetPassword.html || echo "No mustaches found" echo echo "---- First 100 lines of resetPassword.html ----" sed -n '1,100p' backend/core-api/src/private/emailTemplates/resetPassword.html || echo "File not found"
| @@ -0,0 +1 @@ | |||
| {"packageVersion":"0.17.6","sha":"b91386fe3b15d1fbffdc06e265b41875af3c3831","abbreviatedSha":"b91386fe3b","branch":"develop","tag":null,"committer":"Munkh-Orgil <[email protected]>","committerDate":"2020-08-28T07:46:23.000Z","author":"Munkh-Orgil <[email protected]>","authorDate":"2020-08-28T07:46:23.000Z","commitMessage":"erxes/erxes#2253","root":"/home/munkhjin/Documents/workspace/erxes/erxes-api","commonGitDir":"/home/munkhjin/Documents/workspace/erxes/erxes-api/.git","worktreeGitDir":"/home/munkhjin/Documents/workspace/erxes/erxes-api/.git","lastTag":null,"commitsSinceLastTag":null,"parents":["026ff2dd577d52ab010c5044ea88e022a347ec81"]} No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid committing build-time artifacts – make version.json generated, not source-controlled.
version.json contains ephemeral data (commit SHA, branch, absolute paths, etc.). Every new commit will force-change this file, generating noisy diffs and frequent merge conflicts. Prefer generating it during CI/CD or application start-up and add it to .gitignore, rather than tracking it in Git.
🤖 Prompt for AI Agents
In backend/core-api/src/private/version.json at line 1, the version.json file
contains build-time ephemeral data and should not be source-controlled to avoid
noisy diffs and merge conflicts. Remove version.json from the repository, add it
to .gitignore, and implement a process to generate this file dynamically during
CI/CD or application startup instead.
File leaks internal paths and personal e-mail addresses.
The root / commonGitDir fields expose your local directory structure and the author / committer fields expose personal e-mails. If this repo is ever made public or mirrored, that is unnecessary PII leakage. Strip or hash those fields before persisting, or omit them entirely.
🤖 Prompt for AI Agents
In backend/core-api/src/private/version.json at line 1, the file contains
sensitive information including local directory paths in the 'root' and
'commonGitDir' fields and personal email addresses in the 'author' and
'committer' fields. To fix this, remove or replace these fields with anonymized
or hashed values before saving the file to prevent leaking internal paths and
personal data.
| cc: { type: [String], optional: true }, | ||
| bcc: { type: [String], optional: true }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix non-standard Mongoose optional field syntax.
The optional: true syntax is not standard Mongoose. Optional fields in Mongoose are defined by omitting required: true or using required: false.
- cc: { type: [String], optional: true },
- bcc: { type: [String], optional: true },
+ cc: { type: [String] },
+ bcc: { type: [String] },Alternatively, if you want to be explicit about optional fields:
- cc: { type: [String], optional: true },
- bcc: { type: [String], optional: true },
+ cc: { type: [String], required: false },
+ bcc: { type: [String], required: false },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cc: { type: [String], optional: true }, | |
| bcc: { type: [String], optional: true }, | |
| cc: { type: [String] }, | |
| bcc: { type: [String] }, |
| cc: { type: [String], optional: true }, | |
| bcc: { type: [String], optional: true }, | |
| cc: { type: [String], required: false }, | |
| bcc: { type: [String], required: false }, |
🤖 Prompt for AI Agents
In
backend/core-api/src/modules/organization/team-member/db/definitions/emailDeliveries.ts
around lines 10 to 11, the use of 'optional: true' for the cc and bcc fields is
not standard Mongoose syntax. Remove 'optional: true' and ensure these fields
are treated as optional by either omitting the 'required' property or explicitly
setting 'required: false' in their schema definitions.
| <script> | ||
| document.getElementById("year").innerHTML = new Date().getFullYear(); | ||
| </script> No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove inline JavaScript - it won't execute in email clients.
Most email clients block JavaScript execution for security reasons. The copyright year script won't work.
-<script>
- document.getElementById("year").innerHTML = new Date().getFullYear();
-</script>Replace the dynamic year with a handlebars template variable or hardcode the current year:
- Copyright © <span id="year"></span>
+ Copyright © {{ currentYear }}Or simply hardcode it:
- Copyright © <span id="year"></span>
+ Copyright © 2025Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/core-api/src/private/emailTemplates/invitation.html around lines 154
to 156, the inline JavaScript setting the year dynamically will not execute in
email clients. Remove the script tag and replace the dynamic year with a static
value or a handlebars template variable to ensure the year displays correctly in
all email clients.
| <a | ||
| href="{{ content }}" | ||
| target="_blank" | ||
| style=" border-radius: 300px; | ||
| box-shadow: 0 4px 28px rgba(0,0,0,.1); | ||
| transition: .1s ease-in-out; | ||
| transition-property: color,background-color,border-color; | ||
| font-size: 14px; | ||
| /* text-transform: uppercase; */ | ||
| background-color: #63d2d6; | ||
| font-weight: 300; | ||
| text-decoration: none; | ||
| border: 1px solid transparent; | ||
| padding: 8px 40px; | ||
| color: #fff;" | ||
| target="_blank" | ||
| > | ||
| Click here | ||
| </a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove duplicate target attribute.
The anchor tag has a duplicate target="_blank" attribute on lines 131 and 144.
<a
href="{{ content }}"
target="_blank"
style=" border-radius: 300px;
box-shadow: 0 4px 28px rgba(0,0,0,.1);
transition: .1s ease-in-out;
transition-property: color,background-color,border-color;
font-size: 14px;
/* text-transform: uppercase; */
background-color: #63d2d6;
font-weight: 300;
text-decoration: none;
border: 1px solid transparent;
padding: 8px 40px;
color: #fff;"
- target="_blank"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a | |
| href="{{ content }}" | |
| target="_blank" | |
| style=" border-radius: 300px; | |
| box-shadow: 0 4px 28px rgba(0,0,0,.1); | |
| transition: .1s ease-in-out; | |
| transition-property: color,background-color,border-color; | |
| font-size: 14px; | |
| /* text-transform: uppercase; */ | |
| background-color: #63d2d6; | |
| font-weight: 300; | |
| text-decoration: none; | |
| border: 1px solid transparent; | |
| padding: 8px 40px; | |
| color: #fff;" | |
| target="_blank" | |
| > | |
| Click here | |
| </a> | |
| <a | |
| href="{{ content }}" | |
| target="_blank" | |
| style=" border-radius: 300px; | |
| box-shadow: 0 4px 28px rgba(0,0,0,.1); | |
| transition: .1s ease-in-out; | |
| transition-property: color,background-color,border-color; | |
| font-size: 14px; | |
| /* text-transform: uppercase; */ | |
| background-color: #63d2d6; | |
| font-weight: 300; | |
| text-decoration: none; | |
| border: 1px solid transparent; | |
| padding: 8px 40px; | |
| color: #fff;" | |
| > | |
| Click here | |
| </a> |
🧰 Tools
🪛 HTMLHint (1.5.0)
[error] 144-144: Duplicate of attribute name [ target ] was found.
(attr-no-duplication)
🤖 Prompt for AI Agents
In backend/core-api/src/private/emailTemplates/userInvitation.html between lines
129 and 147, the anchor tag has the target="_blank" attribute specified twice.
Remove the duplicate target="_blank" attribute so that it appears only once in
the anchor tag to ensure valid HTML.
| @@ -0,0 +1,268 @@ | |||
| <!DOCTYPE html> | |||
| <head> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add missing title tag for email client compatibility.
The template is missing a <title> tag in the <head> section, which is important for email client compatibility and accessibility.
<head>
+ <title>Team Invitation - erxes</title>
<style type="text/css">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <head> | |
| <head> | |
| <title>Team Invitation - erxes</title> | |
| <style type="text/css"> |
🤖 Prompt for AI Agents
In backend/core-api/src/private/emailTemplates/userInvitation.html at line 2,
the <head> section is missing a <title> tag. Add a <title> tag inside the <head>
element with an appropriate descriptive title for the email to improve email
client compatibility and accessibility.
| } catch (e) { | ||
| // return debugError(e.message); | ||
| console.log(e); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve error handling instead of just logging
Errors are caught and logged but not propagated, which can lead to silent failures.
Consider propagating errors or returning a result object:
} catch (e) {
- // return debugError(e.message);
- console.log(e);
- return;
+ console.error('Failed to create email transporter:', e);
+ throw new Error(`Email configuration error: ${e.message}`);
}And for the sending error:
} catch (e) {
- // debugError(`Error sending email: ${e.message}`);
- console.log(`Error sending email: ${e.message}`);
+ console.error(`Error sending email to ${toEmail}:`, e);
+ if (models && emailDelivery) {
+ await models.EmailDeliveries.updateEmailDelivery(emailDelivery._id, {
+ status: 'failed',
+ error: e.message
+ });
+ }
+ throw e;
}Also applies to: 256-259
🤖 Prompt for AI Agents
In backend/core-api/src/modules/organization/team-member/utils.ts around lines
153 to 157 and similarly at lines 256 to 259, errors are caught and only logged
with console.log, causing silent failures. Modify the catch blocks to either
rethrow the error to propagate it or return a structured result object
indicating failure with error details. This ensures errors are properly handled
and not silently ignored.
| AWS.config.update({ | ||
| region: AWS_REGION, | ||
| accessKeyId: AWS_SES_ACCESS_KEY_ID, | ||
| secretAccessKey: AWS_SES_SECRET_ACCESS_KEY, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid global AWS configuration updates
Updating AWS config globally can affect other AWS services in the application.
Create an SES instance with specific configuration instead:
- AWS.config.update({
- region: AWS_REGION,
- accessKeyId: AWS_SES_ACCESS_KEY_ID,
- secretAccessKey: AWS_SES_SECRET_ACCESS_KEY,
- });
-
return nodemailer.createTransport({
- SES: new AWS.SES({ apiVersion: '2010-12-01' }),
+ SES: new AWS.SES({
+ apiVersion: '2010-12-01',
+ region: AWS_REGION,
+ accessKeyId: AWS_SES_ACCESS_KEY_ID,
+ secretAccessKey: AWS_SES_SECRET_ACCESS_KEY,
+ }),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| AWS.config.update({ | |
| region: AWS_REGION, | |
| accessKeyId: AWS_SES_ACCESS_KEY_ID, | |
| secretAccessKey: AWS_SES_SECRET_ACCESS_KEY, | |
| }); | |
| - AWS.config.update({ | |
| - region: AWS_REGION, | |
| - accessKeyId: AWS_SES_ACCESS_KEY_ID, | |
| - secretAccessKey: AWS_SES_SECRET_ACCESS_KEY, | |
| - }); | |
| - | |
| return nodemailer.createTransport({ | |
| - SES: new AWS.SES({ apiVersion: '2010-12-01' }), | |
| + SES: new AWS.SES({ | |
| + apiVersion: '2010-12-01', | |
| + region: AWS_REGION, | |
| + accessKeyId: AWS_SES_ACCESS_KEY_ID, | |
| + secretAccessKey: AWS_SES_SECRET_ACCESS_KEY, | |
| + }), | |
| }); |
🤖 Prompt for AI Agents
In backend/core-api/src/modules/organization/team-member/utils.ts around lines
28 to 32, avoid updating the global AWS configuration as it can impact other AWS
services. Instead, instantiate a new SES client by passing the region,
accessKeyId, and secretAccessKey directly in its constructor or configuration
object, so the configuration is scoped only to the SES instance.
| ); | ||
|
|
||
| if (transportMethod === 'sendgrid' || (VERSION && VERSION === 'saas')) { | ||
| sendgridMail = require('@sendgrid/mail'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Replace require() with import for consistency
Mixing require() with ES6 imports is inconsistent with TypeScript best practices.
Move the import to the top of the file:
+import * as sendgridMail from '@sendgrid/mail';Then conditionally configure it:
- sendgridMail = require('@sendgrid/mail');
+ // sendgridMail already imported at top🤖 Prompt for AI Agents
In backend/core-api/src/modules/organization/team-member/utils.ts at line 147,
replace the require('@sendgrid/mail') statement with an ES6 import at the top of
the file to maintain consistency with TypeScript best practices. Then,
conditionally configure the imported sendgridMail object as needed within the
code.
| }); | ||
| } | ||
|
|
||
| sendInvitationEmail(models, subdomain, { email: entry.email, token }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for invitation email sending.
The sendInvitationEmail call should be wrapped in error handling to prevent the entire invitation process from failing if email sending fails.
- sendInvitationEmail(models, subdomain, { email: entry.email, token });
+ try {
+ await sendInvitationEmail(models, subdomain, { email: entry.email, token });
+ } catch (error) {
+ console.error('Failed to send invitation email:', error);
+ // Consider whether to continue or throw based on business requirements
+ }🤖 Prompt for AI Agents
In backend/core-api/src/modules/organization/team-member/graphql/mutations.ts at
line 248, the call to sendInvitationEmail lacks error handling, which could
cause the entire invitation process to fail if email sending encounters an
error. Wrap the sendInvitationEmail call in a try-catch block to catch any
exceptions, log or handle the error appropriately, and allow the invitation
process to continue without interruption.
| if (entry.channelIds && (await isEnabled('frontline'))) { | ||
| sendTRPCMessage({ | ||
| pluginName: 'frontline', | ||
| method: 'mutation', | ||
| module: 'inbox', | ||
| action: 'updateUserChannels', | ||
| input: { channelIds: entry.channelIds, userId: createdUser?._id }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for TRPC message sending.
The TRPC message is sent without error handling. If the frontline service is unavailable or the operation fails, it could cause the entire user invitation process to fail silently or throw an unhandled error.
if (entry.channelIds && (await isEnabled('frontline'))) {
- sendTRPCMessage({
- pluginName: 'frontline',
- method: 'mutation',
- module: 'inbox',
- action: 'updateUserChannels',
- input: { channelIds: entry.channelIds, userId: createdUser?._id },
- });
+ try {
+ await sendTRPCMessage({
+ pluginName: 'frontline',
+ method: 'mutation',
+ module: 'inbox',
+ action: 'updateUserChannels',
+ input: { channelIds: entry.channelIds, userId: createdUser?._id },
+ });
+ } catch (error) {
+ console.error('Failed to update user channels:', error);
+ // Continue with invitation process even if channel update fails
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (entry.channelIds && (await isEnabled('frontline'))) { | |
| sendTRPCMessage({ | |
| pluginName: 'frontline', | |
| method: 'mutation', | |
| module: 'inbox', | |
| action: 'updateUserChannels', | |
| input: { channelIds: entry.channelIds, userId: createdUser?._id }, | |
| }); | |
| } | |
| if (entry.channelIds && (await isEnabled('frontline'))) { | |
| try { | |
| await sendTRPCMessage({ | |
| pluginName: 'frontline', | |
| method: 'mutation', | |
| module: 'inbox', | |
| action: 'updateUserChannels', | |
| input: { channelIds: entry.channelIds, userId: createdUser?._id }, | |
| }); | |
| } catch (error) { | |
| console.error('Failed to update user channels:', error); | |
| // Continue with invitation process even if channel update fails | |
| } | |
| } |
🤖 Prompt for AI Agents
In backend/core-api/src/modules/organization/team-member/graphql/mutations.ts
around lines 238 to 246, the sendTRPCMessage call lacks error handling, risking
unhandled exceptions if the frontline service is down or the call fails. Wrap
the sendTRPCMessage invocation in a try-catch block to catch any errors, log or
handle them appropriately, and prevent the failure from affecting the overall
user invitation process.
Summary by Sourcery
Enhance user invitation capabilities by building a robust email delivery system with tracking, template-driven messaging, and frontline integration, while improving related filters, validations, and schema definitions.
New Features:
Bug Fixes:
Enhancements:
Build:
Important
Introduces user invitation feature with email delivery management, new models, utilities, and email templates.
mutations.tswithusersInviteandusersResendInvitation.sendInvitationEmailinutils.tsto send invitation emails.generateFilterinutils.tsto handleidsandexcludeIds.EmailDeliveriesmodel inEmailDeliveries.tsfor managing email delivery records.Usersmodel inUsers.tsto include invitation logic.customer.tsto changeexcludeIdsfromBooleanto[String].EMAIL_DELIVERY_STATUSinconstants.tsfor email status management.handlebarsdependency inpackage.jsonfor email template rendering.This description was created by
for c11e3bb. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores