whatsapp adapter for sending notification#30
whatsapp adapter for sending notification#30ankush-maherwal wants to merge 2 commits intotekdi:mainfrom
Conversation
📝 WalkthroughSummary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe changes add new success and error message constants related to WhatsApp notifications and phone validation. Two new WhatsApp notification adapters are implemented: Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WhatsappAdapter
participant NotificationService
participant WhatsappProvider (Meta API)
Client->>WhatsappAdapter: sendNotification(notificationDataArray)
loop For each notificationData
WhatsappAdapter->>WhatsappAdapter: validate phone & content
WhatsappAdapter->>WhatsappProvider (Meta API): sendViaWhatsappProvider()
WhatsappProvider (Meta API)-->>WhatsappAdapter: success/error response
WhatsappAdapter->>NotificationService: createNotificationLog()
end
WhatsappAdapter-->>Client: return result array
sequenceDiagram
participant Client
participant WhatsappAdapter
participant NotificationService
participant WhatsappProvider (Meta API)
Client->>WhatsappAdapter: sendRawMessages(messageData)
loop For each messageData
WhatsappAdapter->>WhatsappAdapter: validate phone & content
WhatsappAdapter->>WhatsappProvider (Meta API): sendViaWhatsappProvider()
WhatsappProvider (Meta API)-->>WhatsappAdapter: success/error response
WhatsappAdapter->>NotificationService: createNotificationLog()
end
WhatsappAdapter-->>Client: return result array
sequenceDiagram
participant Client
participant WhatsappViaGupshupAdapter
participant NotificationService
participant GupshupAPI
Client->>WhatsappViaGupshupAdapter: sendNotification(notificationDataArray)
loop For each notificationData
WhatsappViaGupshupAdapter->>WhatsappViaGupshupAdapter: validate phone & content
WhatsappViaGupshupAdapter->>GupshupAPI: sendViaGupshupProvider()
GupshupAPI-->>WhatsappViaGupshupAdapter: success/error response
WhatsappViaGupshupAdapter->>NotificationService: createNotificationLog()
end
WhatsappViaGupshupAdapter-->>Client: return result array
sequenceDiagram
participant Client
participant WhatsappViaGupshupAdapter
participant NotificationService
participant GupshupAPI
Client->>WhatsappViaGupshupAdapter: sendRawMessages(messageData)
loop For each messageData
WhatsappViaGupshupAdapter->>WhatsappViaGupshupAdapter: validate phone & content
WhatsappViaGupshupAdapter->>GupshupAPI: sendViaGupshupProvider()
GupshupAPI-->>WhatsappViaGupshupAdapter: success/error response
WhatsappViaGupshupAdapter->>NotificationService: createNotificationLog()
end
WhatsappViaGupshupAdapter-->>Client: return result array
sequenceDiagram
participant Client
participant NotificationAdapterFactory
participant WhatsappAdapter
participant WhatsappViaGupshupAdapter
Client->>NotificationAdapterFactory: getAdapter('whatsapp')
alt Configured provider is Meta
NotificationAdapterFactory->>WhatsappAdapter: return instance
else Configured provider is Gupshup
NotificationAdapterFactory->>WhatsappViaGupshupAdapter: return instance
else Unknown provider
NotificationAdapterFactory->>WhatsappAdapter: return default instance
end
NotificationAdapterFactory-->>Client: adapter instance
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/modules/notification/adapters/whatsappService.adapter.ts (2)
237-247: Replace stub implementation with actual WhatsApp provider integration.The current implementation is a placeholder and needs to be replaced with actual WhatsApp provider logic.
This stub needs to be replaced with real WhatsApp provider integration (e.g., Twilio, Gupshup). The current implementation will always return success, which could mask actual delivery failures.
Would you like me to help generate sample implementations for popular WhatsApp providers like Twilio or provide guidance on the integration pattern?
84-84: Use constant for hardcoded error message.The hardcoded error message should use a constant for consistency.
Apply this diff:
- throw new BadRequestException("Message body is required"); + throw new BadRequestException(ERROR_MESSAGES.NOT_EMPTY_SUBJECT_OR_BODY);Or add a new constant specifically for message body validation if the existing one doesn't fit well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
src/common/utils/constant.util.ts(2 hunks)src/modules/notification/adapters/whatsappService.adapter.ts(1 hunks)
🔇 Additional comments (5)
src/common/utils/constant.util.ts (2)
24-25: LGTM! Good syntax fix and constant addition.The missing comma fix and new WhatsApp success message constant follow the existing conventions properly.
59-61: LGTM! Error constants are well-defined.The syntax fix and new WhatsApp error message constants are appropriate and descriptive.
src/modules/notification/adapters/whatsappService.adapter.ts (3)
20-24: LGTM! Well-defined interface.The
RawWhatsappDatainterface is properly structured with clear field definitions and appropriate optional parameter.
26-31: LGTM! Proper dependency injection setup.The use of
forwardRefto avoid circular dependencies and proper injection of the notification service is well implemented.
151-154: Phone number validation matches E.164 and WhatsApp requirementsThe current regex (
/^\+[1-9]\d{1,14}$/) correctly enforces E.164 format as required by WhatsApp:
- Country codes never start with “0” (so
+0…isn’t valid)- Maximum length of 15 digits is in line with international standards
No changes are needed here.
| to: singleMessageData.to, | ||
| status: 200, | ||
| result: SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY, | ||
| messageId: (result && 'messageId' in result && result.messageId) || (result && 'id' in result && result.id) || `whatsapp-${Date.now()}`, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify complex messageId generation logic.
The nested conditional logic for messageId extraction is overly complex and hard to maintain.
Apply this diff to simplify:
- messageId: (result && 'messageId' in result && result.messageId) || (result && 'id' in result && result.id) || `whatsapp-${Date.now()}`,
+ messageId: result?.messageId || result?.id || `whatsapp-${Date.now()}`,📝 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.
| messageId: (result && 'messageId' in result && result.messageId) || (result && 'id' in result && result.id) || `whatsapp-${Date.now()}`, | |
| messageId: result?.messageId || result?.id || `whatsapp-${Date.now()}`, |
🤖 Prompt for AI Agents
In src/modules/notification/adapters/whatsappService.adapter.ts at line 92, the
messageId assignment uses nested conditionals that are complex and hard to read.
Simplify this by using optional chaining and nullish coalescing operators to
cleanly extract messageId or id from result, falling back to the timestamp
string if neither exists.
| async sendRawMessages(messageData) { | ||
| const results = []; | ||
| const messageDataArray = Array.isArray(messageData) | ||
| ? messageData | ||
| : [messageData]; | ||
| for (const singleMessageData of messageDataArray) { | ||
| try { | ||
| if (!singleMessageData.to || !this.isValidPhone(singleMessageData.to)) { | ||
| throw new BadRequestException(ERROR_MESSAGES.INVALID_PHONE); | ||
| } | ||
| if (!singleMessageData.body) { | ||
| throw new BadRequestException("Message body is required"); | ||
| } | ||
| const result = await this.sendRawMessage(singleMessageData); | ||
| if (result.status === "success") { | ||
| results.push({ | ||
| to: singleMessageData.to, | ||
| status: 200, | ||
| result: SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY, | ||
| messageId: (result && 'messageId' in result && result.messageId) || (result && 'id' in result && result.id) || `whatsapp-${Date.now()}`, | ||
| }); | ||
| } else { | ||
| results.push({ | ||
| to: singleMessageData.to, | ||
| status: 400, | ||
| error: `WhatsApp not sent: ${JSON.stringify(result.errors)}`, | ||
| }); | ||
| } | ||
| } catch (error) { | ||
| LoggerUtil.error(ERROR_MESSAGES.WHATSAPP_NOTIFICATION_FAILED, error); | ||
| results.push({ | ||
| recipient: singleMessageData.to, | ||
| status: 500, | ||
| error: error.message || error.toString(), | ||
| }); | ||
| } | ||
| } | ||
| return results; | ||
| } |
There was a problem hiding this comment.
Fix inconsistent status codes and improve type annotations.
Similar to the previous method, this has inconsistent status representations and missing type annotations.
Apply this diff for consistency:
- async sendRawMessages(messageData) {
+ async sendRawMessages(messageData: RawWhatsappData | RawWhatsappData[]): Promise<any[]> {
const results = [];
const messageDataArray = Array.isArray(messageData)
? messageData
: [messageData];
for (const singleMessageData of messageDataArray) {
try {
if (!singleMessageData.to || !this.isValidPhone(singleMessageData.to)) {
throw new BadRequestException(ERROR_MESSAGES.INVALID_PHONE);
}
if (!singleMessageData.body) {
throw new BadRequestException("Message body is required");
}
const result = await this.sendRawMessage(singleMessageData);
if (result.status === "success") {
results.push({
to: singleMessageData.to,
- status: 200,
+ status: "success",
result: SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY,
messageId: (result && 'messageId' in result && result.messageId) || (result && 'id' in result && result.id) || `whatsapp-${Date.now()}`,
});
} else {
results.push({
to: singleMessageData.to,
- status: 400,
+ status: "error",
error: `WhatsApp not sent: ${JSON.stringify(result.errors)}`,
});
}
} catch (error) {
LoggerUtil.error(ERROR_MESSAGES.WHATSAPP_NOTIFICATION_FAILED, error);
results.push({
recipient: singleMessageData.to,
- status: 500,
+ status: "error",
error: error.message || error.toString(),
});
}
}
return results;
}📝 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.
| async sendRawMessages(messageData) { | |
| const results = []; | |
| const messageDataArray = Array.isArray(messageData) | |
| ? messageData | |
| : [messageData]; | |
| for (const singleMessageData of messageDataArray) { | |
| try { | |
| if (!singleMessageData.to || !this.isValidPhone(singleMessageData.to)) { | |
| throw new BadRequestException(ERROR_MESSAGES.INVALID_PHONE); | |
| } | |
| if (!singleMessageData.body) { | |
| throw new BadRequestException("Message body is required"); | |
| } | |
| const result = await this.sendRawMessage(singleMessageData); | |
| if (result.status === "success") { | |
| results.push({ | |
| to: singleMessageData.to, | |
| status: 200, | |
| result: SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY, | |
| messageId: (result && 'messageId' in result && result.messageId) || (result && 'id' in result && result.id) || `whatsapp-${Date.now()}`, | |
| }); | |
| } else { | |
| results.push({ | |
| to: singleMessageData.to, | |
| status: 400, | |
| error: `WhatsApp not sent: ${JSON.stringify(result.errors)}`, | |
| }); | |
| } | |
| } catch (error) { | |
| LoggerUtil.error(ERROR_MESSAGES.WHATSAPP_NOTIFICATION_FAILED, error); | |
| results.push({ | |
| recipient: singleMessageData.to, | |
| status: 500, | |
| error: error.message || error.toString(), | |
| }); | |
| } | |
| } | |
| return results; | |
| } | |
| async sendRawMessages(messageData: RawWhatsappData | RawWhatsappData[]): Promise<any[]> { | |
| const results = []; | |
| const messageDataArray = Array.isArray(messageData) | |
| ? messageData | |
| : [messageData]; | |
| for (const singleMessageData of messageDataArray) { | |
| try { | |
| if (!singleMessageData.to || !this.isValidPhone(singleMessageData.to)) { | |
| throw new BadRequestException(ERROR_MESSAGES.INVALID_PHONE); | |
| } | |
| if (!singleMessageData.body) { | |
| throw new BadRequestException("Message body is required"); | |
| } | |
| const result = await this.sendRawMessage(singleMessageData); | |
| if (result.status === "success") { | |
| results.push({ | |
| to: singleMessageData.to, | |
| status: "success", | |
| result: SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY, | |
| messageId: | |
| (result && 'messageId' in result && result.messageId) || | |
| (result && 'id' in result && result.id) || | |
| `whatsapp-${Date.now()}`, | |
| }); | |
| } else { | |
| results.push({ | |
| to: singleMessageData.to, | |
| status: "error", | |
| error: `WhatsApp not sent: ${JSON.stringify(result.errors)}`, | |
| }); | |
| } | |
| } catch (error) { | |
| LoggerUtil.error(ERROR_MESSAGES.WHATSAPP_NOTIFICATION_FAILED, error); | |
| results.push({ | |
| recipient: singleMessageData.to, | |
| status: "error", | |
| error: error.message || error.toString(), | |
| }); | |
| } | |
| } | |
| return results; | |
| } |
🤖 Prompt for AI Agents
In src/modules/notification/adapters/whatsappService.adapter.ts lines 73 to 111,
the sendRawMessages method has inconsistent status code usage (mixing numeric
and string statuses) and lacks proper type annotations for the messageData
parameter and the results array. Fix this by ensuring all status codes are
consistently numeric (e.g., 200 for success, 400 for client errors, 500 for
server errors) and add appropriate TypeScript type annotations to the method
parameters and return type to improve type safety and clarity.
| async sendNotification(notificationDataArray) { | ||
| const results = []; | ||
| for (const notificationData of notificationDataArray) { | ||
| try { | ||
| const recipient = notificationData.recipient; | ||
| if (!recipient || !this.isValidPhone(recipient)) { | ||
| throw new BadRequestException(ERROR_MESSAGES.INVALID_PHONE); | ||
| } | ||
| const result = await this.send(notificationData); | ||
| if (result.status === "success") { | ||
| results.push({ | ||
| recipient: recipient, | ||
| status: 200, | ||
| result: SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY, | ||
| }); | ||
| } else { | ||
| results.push({ | ||
| recipient: recipient, | ||
| status: "error", | ||
| error: `WhatsApp not sent: ${JSON.stringify(result.errors)}`, | ||
| }); | ||
| } | ||
| } catch (error) { | ||
| LoggerUtil.error(ERROR_MESSAGES.WHATSAPP_NOTIFICATION_FAILED, error); | ||
| results.push({ | ||
| recipient: notificationData.recipient, | ||
| status: "error", | ||
| error: error.toString(), | ||
| }); | ||
| } | ||
| } | ||
| return results; | ||
| } |
There was a problem hiding this comment.
Fix inconsistent status values and improve type annotations.
The method has inconsistent status representations and missing type annotations.
Apply this diff to improve consistency and type safety:
- async sendNotification(notificationDataArray) {
+ async sendNotification(notificationDataArray: any[]): Promise<any[]> {
const results = [];
for (const notificationData of notificationDataArray) {
try {
const recipient = notificationData.recipient;
if (!recipient || !this.isValidPhone(recipient)) {
throw new BadRequestException(ERROR_MESSAGES.INVALID_PHONE);
}
const result = await this.send(notificationData);
if (result.status === "success") {
results.push({
recipient: recipient,
- status: 200,
+ status: "success",
result: SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY,
});
} else {
results.push({
recipient: recipient,
status: "error",
error: `WhatsApp not sent: ${JSON.stringify(result.errors)}`,
});
}
} catch (error) {
LoggerUtil.error(ERROR_MESSAGES.WHATSAPP_NOTIFICATION_FAILED, error);
results.push({
recipient: notificationData.recipient,
status: "error",
error: error.toString(),
});
}
}
return results;
}📝 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.
| async sendNotification(notificationDataArray) { | |
| const results = []; | |
| for (const notificationData of notificationDataArray) { | |
| try { | |
| const recipient = notificationData.recipient; | |
| if (!recipient || !this.isValidPhone(recipient)) { | |
| throw new BadRequestException(ERROR_MESSAGES.INVALID_PHONE); | |
| } | |
| const result = await this.send(notificationData); | |
| if (result.status === "success") { | |
| results.push({ | |
| recipient: recipient, | |
| status: 200, | |
| result: SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY, | |
| }); | |
| } else { | |
| results.push({ | |
| recipient: recipient, | |
| status: "error", | |
| error: `WhatsApp not sent: ${JSON.stringify(result.errors)}`, | |
| }); | |
| } | |
| } catch (error) { | |
| LoggerUtil.error(ERROR_MESSAGES.WHATSAPP_NOTIFICATION_FAILED, error); | |
| results.push({ | |
| recipient: notificationData.recipient, | |
| status: "error", | |
| error: error.toString(), | |
| }); | |
| } | |
| } | |
| return results; | |
| } | |
| async sendNotification(notificationDataArray: any[]): Promise<any[]> { | |
| const results = []; | |
| for (const notificationData of notificationDataArray) { | |
| try { | |
| const recipient = notificationData.recipient; | |
| if (!recipient || !this.isValidPhone(recipient)) { | |
| throw new BadRequestException(ERROR_MESSAGES.INVALID_PHONE); | |
| } | |
| const result = await this.send(notificationData); | |
| if (result.status === "success") { | |
| results.push({ | |
| recipient: recipient, | |
| status: "success", | |
| result: SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY, | |
| }); | |
| } else { | |
| results.push({ | |
| recipient: recipient, | |
| status: "error", | |
| error: `WhatsApp not sent: ${JSON.stringify(result.errors)}`, | |
| }); | |
| } | |
| } catch (error) { | |
| LoggerUtil.error(ERROR_MESSAGES.WHATSAPP_NOTIFICATION_FAILED, error); | |
| results.push({ | |
| recipient: notificationData.recipient, | |
| status: "error", | |
| error: error.toString(), | |
| }); | |
| } | |
| } | |
| return results; | |
| } |
🤖 Prompt for AI Agents
In src/modules/notification/adapters/whatsappService.adapter.ts between lines 36
and 68, the sendNotification method uses inconsistent status values (mixing
number 200 and string "error") and lacks type annotations. To fix this,
standardize the status field to use consistent types (e.g., use string values
like "success" and "error" consistently) and add appropriate TypeScript type
annotations for the method parameters and return type to improve type safety and
clarity.
| async send(notificationData) { | ||
| const notificationLogs = this.createNotificationLog( | ||
| notificationData, | ||
| notificationData.body, | ||
| notificationData.recipient | ||
| ); | ||
| try { | ||
| // Replace this with your WhatsApp provider logic (e.g., Twilio, Gupshup, etc.) | ||
| const result = await this.sendViaWhatsappProvider({ | ||
| to: notificationData.recipient, | ||
| body: notificationData.body, | ||
| from: process.env.WHATSAPP_FROM, | ||
| }); | ||
| if (result.status === "success") { | ||
| notificationLogs.status = true; | ||
| await this.notificationServices.saveNotificationLogs(notificationLogs); | ||
| LoggerUtil.log( | ||
| SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY | ||
| ); | ||
| return result; | ||
| } else { | ||
| throw new Error(`WhatsApp not sent: ${JSON.stringify(result.errors)}`); | ||
| } | ||
| } catch (e) { | ||
| LoggerUtil.error(ERROR_MESSAGES.WHATSAPP_NOTIFICATION_FAILED, e); | ||
| notificationLogs.status = false; | ||
| notificationLogs.error = e.toString(); | ||
| await this.notificationServices.saveNotificationLogs(notificationLogs); | ||
| return { | ||
| status: "error", | ||
| errors: [e.message || e.toString()], | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type annotations for better type safety.
The method parameters and return types should be properly typed.
Apply this diff:
- async send(notificationData) {
+ async send(notificationData: any): Promise<any> {📝 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.
| async send(notificationData) { | |
| const notificationLogs = this.createNotificationLog( | |
| notificationData, | |
| notificationData.body, | |
| notificationData.recipient | |
| ); | |
| try { | |
| // Replace this with your WhatsApp provider logic (e.g., Twilio, Gupshup, etc.) | |
| const result = await this.sendViaWhatsappProvider({ | |
| to: notificationData.recipient, | |
| body: notificationData.body, | |
| from: process.env.WHATSAPP_FROM, | |
| }); | |
| if (result.status === "success") { | |
| notificationLogs.status = true; | |
| await this.notificationServices.saveNotificationLogs(notificationLogs); | |
| LoggerUtil.log( | |
| SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY | |
| ); | |
| return result; | |
| } else { | |
| throw new Error(`WhatsApp not sent: ${JSON.stringify(result.errors)}`); | |
| } | |
| } catch (e) { | |
| LoggerUtil.error(ERROR_MESSAGES.WHATSAPP_NOTIFICATION_FAILED, e); | |
| notificationLogs.status = false; | |
| notificationLogs.error = e.toString(); | |
| await this.notificationServices.saveNotificationLogs(notificationLogs); | |
| return { | |
| status: "error", | |
| errors: [e.message || e.toString()], | |
| }; | |
| } | |
| } | |
| async send(notificationData: any): Promise<any> { | |
| const notificationLogs = this.createNotificationLog( | |
| notificationData, | |
| notificationData.body, | |
| notificationData.recipient | |
| ); | |
| try { | |
| // Replace this with your WhatsApp provider logic (e.g., Twilio, Gupshup, etc.) | |
| const result = await this.sendViaWhatsappProvider({ | |
| to: notificationData.recipient, | |
| body: notificationData.body, | |
| from: process.env.WHATSAPP_FROM, | |
| }); | |
| if (result.status === "success") { | |
| notificationLogs.status = true; | |
| await this.notificationServices.saveNotificationLogs(notificationLogs); | |
| LoggerUtil.log( | |
| SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY | |
| ); | |
| return result; | |
| } else { | |
| throw new Error(`WhatsApp not sent: ${JSON.stringify(result.errors)}`); | |
| } | |
| } catch (e) { | |
| LoggerUtil.error(ERROR_MESSAGES.WHATSAPP_NOTIFICATION_FAILED, e); | |
| notificationLogs.status = false; | |
| notificationLogs.error = e.toString(); | |
| await this.notificationServices.saveNotificationLogs(notificationLogs); | |
| return { | |
| status: "error", | |
| errors: [e.message || e.toString()], | |
| }; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/modules/notification/adapters/whatsappService.adapter.ts between lines
159 and 192, the send method lacks type annotations for its parameter and return
type. Add appropriate TypeScript type annotations to the notificationData
parameter and specify the return type of the send method to improve type safety
and clarity.
|
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (4)
src/modules/notification/adapters/whatsappService.adapter.ts (4)
59-91: Fix inconsistent status values and improve type annotations.The method has inconsistent status representations and missing type annotations.
96-134: Fix inconsistent status codes and improve type annotations.Similar to the previous method, this has inconsistent status representations and missing type annotations.
115-115: Simplify complex messageId generation logic.The nested conditional logic for messageId extraction is overly complex and hard to maintain.
182-214: Add type annotations for better type safety.The method parameters and return types should be properly typed.
🧹 Nitpick comments (1)
src/modules/notification/adapters/whatsappService.adapter.ts (1)
20-27: Consider moving interface to a shared types file.Since both WhatsApp adapters use similar data structures, consider creating a shared types file to avoid duplication and ensure consistency across adapters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
GUPSHUP_WHATSAPP_SETUP.md(1 hunks)WHATSAPP_ADAPTERS_SUMMARY.md(1 hunks)WHATSAPP_SETUP.md(1 hunks)package.json(2 hunks)src/common/utils/constant.util.ts(2 hunks)src/modules/notification/adapters/whatsappService.adapter.ts(1 hunks)src/modules/notification/adapters/whatsappViaGupshup.adapter.ts(1 hunks)src/modules/notification/notification.module.ts(2 hunks)src/modules/notification/notificationadapters.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/common/utils/constant.util.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/modules/notification/notificationadapters.ts (2)
src/common/utils/constant.util.ts (1)
WHATSAPP_PROVIDER(70-74)src/modules/notification/interface/notificationService.ts (1)
NotificationServiceInterface(1-3)
🪛 LanguageTool
GUPSHUP_WHATSAPP_SETUP.md
[uncategorized] ~154-~154: Loose punctuation mark.
Context: ... } } ``` ## Common Error Codes - 400: Bad Request - Invalid parameters - `401...
(UNLIKELY_OPENING_PUNCTUATION)
WHATSAPP_ADAPTERS_SUMMARY.md
[style] ~159-~159: Specify a number, remove phrase, use “a few”, or use “some”
Context: ...t rate limits 4. Testing: Test with a small number of messages before going live 5. **Monitor...
(SMALL_NUMBER_OF)
WHATSAPP_SETUP.md
[uncategorized] ~77-~77: Loose punctuation mark.
Context: ...ta); ``` ## Common Error Codes - 100: Invalid phone number format - `131`: Me...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (9)
WHATSAPP_SETUP.md (1)
1-102: Comprehensive and well-structured setup documentation.The WhatsApp Business API setup guide is thorough and covers all essential aspects:
- Clear prerequisites and step-by-step credential setup
- Proper environment variable configuration
- Practical usage examples with correct API patterns
- Security best practices and troubleshooting guidance
The documentation aligns well with the adapter implementation and provides users with actionable setup instructions.
src/modules/notification/notification.module.ts (2)
9-10: Clean adapter imports.The import statements for both WhatsApp adapters are correctly structured and follow the established pattern for other adapters in the module.
25-34: Proper provider registration.Both WhatsApp adapters are correctly added to the providers array, enabling dependency injection throughout the notification module. The registration follows NestJS best practices.
src/modules/notification/notificationadapters.ts (4)
6-9: Clean imports and dependency setup.The imports for both WhatsApp adapters, ConfigService, and constants are properly organized and follow the established pattern.
13-24: Proper configuration initialization.The whatsappProvider configuration is correctly initialized with a sensible default fallback to META provider, ensuring the factory remains functional even without explicit configuration.
34-40: Sound provider selection logic.The WhatsApp case in getAdapter method correctly implements provider-based selection with a clear fallback to the Meta adapter, ensuring reliable operation.
49-61: Mapping of TWILIO to genericwhatsappAdapteris intentional
We searched the codebase for a Twilio-specific WhatsApp adapter and found none—both META and TWILIO cases correctly fall back to the existingwhatsappAdapter. No changes are needed here unless you plan to introduce a dedicated Twilio implementation.• File: src/modules/notification/notificationadapters.ts
Lines: 49–61 (getWhatsappAdapter)WHATSAPP_ADAPTERS_SUMMARY.md (1)
1-175: Excellent comprehensive adapter comparison guide.This summary document provides outstanding value by:
- Clearly distinguishing between the two WhatsApp adapters and their use cases
- Providing accurate environment configuration examples
- Demonstrating proper usage patterns with the factory
- Including a detailed feature comparison table
- Offering practical migration instructions
The documentation will significantly help users make informed decisions about which adapter to use and how to implement it correctly.
GUPSHUP_WHATSAPP_SETUP.md (1)
1-245: Comprehensive and technically accurate Gupshup setup guide.The documentation is excellent and provides:
- Clear step-by-step credential acquisition process
- Accurate API endpoint and payload documentation
- Practical usage examples for different message types
- Proper security considerations and best practices
- Helpful troubleshooting section
The API payload structures and endpoints appear correct based on standard Gupshup API patterns, and the integration instructions align well with the adapter implementation.
| const whatsappAccessToken = this.configService.get('WHATSAPP_ACCESS_TOKEN'); | ||
| const whatsappPhoneNumberId = this.configService.get('WHATSAPP_PHONE_NUMBER_ID'); | ||
| const whatsappApiVersion = this.configService.get('WHATSAPP_API_VERSION', 'v18.0'); | ||
|
|
||
| if (!whatsappAccessToken || !whatsappPhoneNumberId) { | ||
| throw new Error('WhatsApp Business API credentials not configured. Please set WHATSAPP_ACCESS_TOKEN and WHATSAPP_PHONE_NUMBER_ID environment variables.'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicate configuration retrieval.
The WhatsApp configuration values are already initialized in the constructor but are being retrieved again here. This creates unnecessary duplication and potential inconsistency.
Use the class properties instead:
- // Get WhatsApp credentials from environment
- const whatsappAccessToken = this.configService.get('WHATSAPP_ACCESS_TOKEN');
- const whatsappPhoneNumberId = this.configService.get('WHATSAPP_PHONE_NUMBER_ID');
- const whatsappApiVersion = this.configService.get('WHATSAPP_API_VERSION', 'v18.0');
-
- if (!whatsappAccessToken || !whatsappPhoneNumberId) {
+ if (!this.whatsappAccessToken || !this.whatsappPhoneNumberId) {
throw new Error('WhatsApp Business API credentials not configured. Please set WHATSAPP_ACCESS_TOKEN and WHATSAPP_PHONE_NUMBER_ID environment variables.');
}And update the API URL construction:
- const apiUrl = `https://graph.facebook.com/${whatsappApiVersion}/${whatsappPhoneNumberId}/messages`;
+ const apiUrl = `${this.whatsappApiBaseUrl}/${this.whatsappPhoneNumberId}/messages`;And the authorization header:
- 'Authorization': `Bearer ${whatsappAccessToken}`,
+ 'Authorization': `Bearer ${this.whatsappAccessToken}`,📝 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.
| const whatsappAccessToken = this.configService.get('WHATSAPP_ACCESS_TOKEN'); | |
| const whatsappPhoneNumberId = this.configService.get('WHATSAPP_PHONE_NUMBER_ID'); | |
| const whatsappApiVersion = this.configService.get('WHATSAPP_API_VERSION', 'v18.0'); | |
| if (!whatsappAccessToken || !whatsappPhoneNumberId) { | |
| throw new Error('WhatsApp Business API credentials not configured. Please set WHATSAPP_ACCESS_TOKEN and WHATSAPP_PHONE_NUMBER_ID environment variables.'); | |
| } | |
| // … earlier context … | |
| // Use class properties instead of fetching from configService again | |
| if (!this.whatsappAccessToken || !this.whatsappPhoneNumberId) { | |
| throw new Error( | |
| 'WhatsApp Business API credentials not configured. Please set WHATSAPP_ACCESS_TOKEN and WHATSAPP_PHONE_NUMBER_ID environment variables.' | |
| ); | |
| } | |
| // Build URL using the base URL already constructed in constructor | |
| const apiUrl = `${this.whatsappApiBaseUrl}/${this.whatsappPhoneNumberId}/messages`; | |
| // Send request with the stored access token | |
| const response = await this.httpService | |
| .post( | |
| apiUrl, | |
| /* payload */, | |
| { | |
| headers: { | |
| Authorization: `Bearer ${this.whatsappAccessToken}`, | |
| 'Content-Type': 'application/json', | |
| }, | |
| }, | |
| ) | |
| .toPromise(); | |
| // … remaining code … |
🤖 Prompt for AI Agents
In src/modules/notification/adapters/whatsappService.adapter.ts around lines 261
to 267, the WhatsApp configuration values are redundantly retrieved from
configService instead of using the class properties initialized in the
constructor. Remove the duplicate retrieval of whatsappAccessToken,
whatsappPhoneNumberId, and whatsappApiVersion here and replace their usage with
the corresponding class properties. Also, update the API URL construction and
authorization header to use these class properties to avoid inconsistency and
duplication.
| if (from) { | ||
| messagePayload.source = from; | ||
| } |
There was a problem hiding this comment.
Avoid overriding configured source parameter.
When a custom from parameter is provided, it completely overrides the configured source instead of being validated or used as a fallback.
This could lead to messages being sent from unauthorized sources. Consider validating the from parameter against allowed sources or using it only when the configured source is not set.
🤖 Prompt for AI Agents
In src/modules/notification/adapters/whatsappViaGupshup.adapter.ts around lines
300 to 302, the code overrides the configured source with the provided from
parameter without validation, risking unauthorized sources. Modify the logic to
validate the from parameter against a list of allowed sources before assigning
it to messagePayload.source, or use from only as a fallback if the configured
source is not set.
| this.gupshupApiKey = this.configService.get('GUPSHUP_API_KEY'); | ||
| this.gupshupChannelId = this.configService.get('GUPSHUP_CHANNEL_ID'); | ||
| this.gupshupSource = this.configService.get('GUPSHUP_SOURCE'); | ||
| this.gupshupApiUrl = this.configService.get('GUPSHUP_API_URL', 'https://api.gupshup.io/wa/api/v1'); | ||
|
|
||
| if (!this.gupshupApiKey || !this.gupshupChannelId || !this.gupshupSource) { | ||
| LoggerUtil.error('Gupshup WhatsApp API credentials not configured properly'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider implementing a configuration validation method.
Both WhatsApp adapters check for missing credentials in the constructor but only log errors. This could lead to runtime failures.
Consider adding a validation method that throws an exception during initialization if required credentials are missing, preventing the service from starting in an invalid state. This fail-fast approach is better than discovering configuration issues at runtime.
🤖 Prompt for AI Agents
In src/modules/notification/adapters/whatsappViaGupshup.adapter.ts around lines
54 to 61, the constructor currently only logs an error if required Gupshup API
credentials are missing, which can cause runtime failures. Implement a dedicated
configuration validation method that checks for all required credentials and
throws an exception if any are missing. Call this validation method during
initialization to enforce a fail-fast approach, preventing the service from
starting with invalid or incomplete configuration.
| export interface RawWhatsappGupshupData { | ||
| to: string; // WhatsApp number in international format | ||
| body: string; | ||
| from?: string; | ||
| templateId?: string; // For template messages | ||
| templateParams?: any[]; // For template parameters | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clarify the purpose of template fields in RawWhatsappGupshupData.
The interface includes templateId and templateParams fields, but the sendRawMessage method name suggests it's for non-template messages. This creates confusion about when to use which method.
Consider either:
- Removing template fields from this interface and using them only in
sendTemplateMessage, or - Renaming the interface to reflect its dual purpose
🤖 Prompt for AI Agents
In src/modules/notification/adapters/whatsappViaGupshup.adapter.ts around lines
23 to 29, the RawWhatsappGupshupData interface includes templateId and
templateParams fields which conflicts with the sendRawMessage method's
non-template message purpose. To fix this, either remove the templateId and
templateParams fields from this interface and restrict them to a separate
interface used only by sendTemplateMessage, or rename RawWhatsappGupshupData to
a name that clearly indicates it supports both raw and template messages to
avoid confusion.
| async sendTemplateMessage(templateData: { | ||
| to: string; | ||
| templateId: string; | ||
| templateParams: any[]; | ||
| from?: string; | ||
| }) { | ||
| const notificationLogs = this.createRawNotificationLog( | ||
| `Template: ${templateData.templateId}`, | ||
| templateData.to | ||
| ); | ||
|
|
||
| try { | ||
| const result = await this.sendViaGupshupProvider({ | ||
| to: templateData.to, | ||
| body: '', // Not used for templates | ||
| from: templateData.from || this.configService.get('WHATSAPP_FROM'), | ||
| templateId: templateData.templateId, | ||
| templateParams: templateData.templateParams, | ||
| }); | ||
|
|
||
| if (result.status === "success") { | ||
| notificationLogs.status = true; | ||
| await this.notificationServices.saveNotificationLogs(notificationLogs); | ||
| LoggerUtil.log('WhatsApp template message sent successfully via Gupshup'); | ||
| return { | ||
| ...result, | ||
| messageId: result.id || `gupshup-template-${Date.now()}`, | ||
| }; | ||
| } else { | ||
| throw new Error(`WhatsApp template not sent: ${JSON.stringify(result.errors)}`); | ||
| } | ||
| } catch (e) { | ||
| LoggerUtil.error('WhatsApp template message failed:', e); | ||
| notificationLogs.status = false; | ||
| notificationLogs.error = e.toString(); | ||
| await this.notificationServices.saveNotificationLogs(notificationLogs); | ||
| return { | ||
| status: "error", | ||
| errors: [e.message || e.toString()], | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add proper type annotations and input validation for template messages.
The sendTemplateMessage method lacks proper TypeScript typing and doesn't validate the template parameters.
Add a proper interface and validation:
interface TemplateMessageData {
to: string;
templateId: string;
templateParams: any[];
from?: string;
}
async sendTemplateMessage(templateData: TemplateMessageData): Promise<any> {
// Validate phone number
if (!this.isValidPhone(templateData.to)) {
throw new BadRequestException(ERROR_MESSAGES.INVALID_PHONE);
}
// Validate template parameters
if (!Array.isArray(templateData.templateParams)) {
throw new BadRequestException("Template parameters must be an array");
}
// ... rest of the implementation
}🤖 Prompt for AI Agents
In src/modules/notification/adapters/whatsappViaGupshup.adapter.ts lines 365 to
406, the sendTemplateMessage method lacks proper TypeScript type annotations and
input validation. Define a TemplateMessageData interface for the method
parameter to enforce typing. Add validation to check if the 'to' phone number is
valid using this.isValidPhone and throw a BadRequestException with an
appropriate error message if invalid. Also validate that templateParams is an
array and throw a BadRequestException if not. Incorporate these checks at the
start of the method before proceeding with sending the message.
| async sendRawMessages(messageData) { | ||
| const results = []; | ||
| const messageDataArray = Array.isArray(messageData) | ||
| ? messageData | ||
| : [messageData]; | ||
| for (const singleMessageData of messageDataArray) { | ||
| try { | ||
| if (!singleMessageData.to || !this.isValidPhone(singleMessageData.to)) { | ||
| throw new BadRequestException(ERROR_MESSAGES.INVALID_PHONE); | ||
| } | ||
| if (!singleMessageData.body) { | ||
| throw new BadRequestException("Message body is required"); | ||
| } | ||
| const result = await this.sendRawMessage(singleMessageData); | ||
| if (result.status === "success") { | ||
| results.push({ | ||
| to: singleMessageData.to, | ||
| status: 200, | ||
| result: SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY, | ||
| messageId: (result && 'messageId' in result && result.messageId) || (result && 'id' in result && result.id) || `whatsapp-gupshup-${Date.now()}`, | ||
| }); | ||
| } else { | ||
| results.push({ | ||
| to: singleMessageData.to, | ||
| status: 400, | ||
| error: `WhatsApp not sent: ${JSON.stringify(result.errors)}`, | ||
| }); | ||
| } | ||
| } catch (error) { | ||
| LoggerUtil.error(ERROR_MESSAGES.WHATSAPP_NOTIFICATION_FAILED, error); | ||
| results.push({ | ||
| recipient: singleMessageData.to, | ||
| status: 500, | ||
| error: error.message || error.toString(), | ||
| }); | ||
| } | ||
| } | ||
| return results; | ||
| } |
There was a problem hiding this comment.
Fix inconsistent status codes and improve type annotations.
Similar inconsistent status representations (200, 400, 500 mixed with "error") and missing type annotations.
🤖 Prompt for AI Agents
In src/modules/notification/adapters/whatsappViaGupshup.adapter.ts between lines
104 and 142, the status codes and error fields in the results array are
inconsistently used, mixing numeric and string types, and the method lacks
proper type annotations. Standardize the status codes to always be numeric
(e.g., 200 for success, 400 for client errors, 500 for server errors) and ensure
error fields are consistently named and typed. Additionally, add appropriate
TypeScript type annotations for the method parameters and return type to improve
type safety and clarity.
| to: singleMessageData.to, | ||
| status: 200, | ||
| result: SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY, | ||
| messageId: (result && 'messageId' in result && result.messageId) || (result && 'id' in result && result.id) || `whatsapp-gupshup-${Date.now()}`, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify complex messageId generation logic.
The same overly complex nested conditional logic for messageId extraction.
Apply this diff:
- messageId: (result && 'messageId' in result && result.messageId) || (result && 'id' in result && result.id) || `whatsapp-gupshup-${Date.now()}`,
+ messageId: result?.messageId || result?.id || `whatsapp-gupshup-${Date.now()}`,🤖 Prompt for AI Agents
In src/modules/notification/adapters/whatsappViaGupshup.adapter.ts at line 123,
simplify the messageId assignment by removing the nested conditional checks. Use
a clearer approach such as optional chaining and nullish coalescing to extract
messageId or id from result, falling back to the timestamp string if neither
exists.
| async sendNotification(notificationDataArray) { | ||
| const results = []; | ||
| for (const notificationData of notificationDataArray) { | ||
| try { | ||
| const recipient = notificationData.recipient; | ||
| if (!recipient || !this.isValidPhone(recipient)) { | ||
| throw new BadRequestException(ERROR_MESSAGES.INVALID_PHONE); | ||
| } | ||
| const result = await this.send(notificationData); | ||
| if (result.status === "success") { | ||
| results.push({ | ||
| recipient: recipient, | ||
| status: 200, | ||
| result: SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY, | ||
| }); | ||
| } else { | ||
| results.push({ | ||
| recipient: recipient, | ||
| status: "error", | ||
| error: `WhatsApp not sent: ${JSON.stringify(result.errors)}`, | ||
| }); | ||
| } | ||
| } catch (error) { | ||
| LoggerUtil.error(ERROR_MESSAGES.WHATSAPP_NOTIFICATION_FAILED, error); | ||
| results.push({ | ||
| recipient: notificationData.recipient, | ||
| status: "error", | ||
| error: error.toString(), | ||
| }); | ||
| } | ||
| } | ||
| return results; | ||
| } |
There was a problem hiding this comment.
Fix inconsistent status values and improve type annotations.
The method has the same inconsistent status representations and missing type annotations as the Meta WhatsApp adapter.
Apply consistent status values and add proper type annotations:
- async sendNotification(notificationDataArray) {
+ async sendNotification(notificationDataArray: any[]): Promise<any[]> {And ensure consistent status values throughout (either use HTTP codes or strings, not both).
📝 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.
| async sendNotification(notificationDataArray) { | |
| const results = []; | |
| for (const notificationData of notificationDataArray) { | |
| try { | |
| const recipient = notificationData.recipient; | |
| if (!recipient || !this.isValidPhone(recipient)) { | |
| throw new BadRequestException(ERROR_MESSAGES.INVALID_PHONE); | |
| } | |
| const result = await this.send(notificationData); | |
| if (result.status === "success") { | |
| results.push({ | |
| recipient: recipient, | |
| status: 200, | |
| result: SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY, | |
| }); | |
| } else { | |
| results.push({ | |
| recipient: recipient, | |
| status: "error", | |
| error: `WhatsApp not sent: ${JSON.stringify(result.errors)}`, | |
| }); | |
| } | |
| } catch (error) { | |
| LoggerUtil.error(ERROR_MESSAGES.WHATSAPP_NOTIFICATION_FAILED, error); | |
| results.push({ | |
| recipient: notificationData.recipient, | |
| status: "error", | |
| error: error.toString(), | |
| }); | |
| } | |
| } | |
| return results; | |
| } | |
| async sendNotification(notificationDataArray: any[]): Promise<any[]> { | |
| const results = []; | |
| for (const notificationData of notificationDataArray) { | |
| try { | |
| const recipient = notificationData.recipient; | |
| if (!recipient || !this.isValidPhone(recipient)) { | |
| throw new BadRequestException(ERROR_MESSAGES.INVALID_PHONE); | |
| } | |
| const result = await this.send(notificationData); | |
| if (result.status === "success") { | |
| results.push({ | |
| recipient: recipient, | |
| status: 200, | |
| result: SUCCESS_MESSAGES.WHATSAPP_NOTIFICATION_SEND_SUCCESSFULLY, | |
| }); | |
| } else { | |
| results.push({ | |
| recipient: recipient, | |
| status: "error", | |
| error: `WhatsApp not sent: ${JSON.stringify(result.errors)}`, | |
| }); | |
| } | |
| } catch (error) { | |
| LoggerUtil.error(ERROR_MESSAGES.WHATSAPP_NOTIFICATION_FAILED, error); | |
| results.push({ | |
| recipient: notificationData.recipient, | |
| status: "error", | |
| error: error.toString(), | |
| }); | |
| } | |
| } | |
| return results; | |
| } |
🤖 Prompt for AI Agents
In src/modules/notification/adapters/whatsappViaGupshup.adapter.ts between lines
67 and 99, the sendNotification method uses inconsistent status values mixing
HTTP codes (200) and strings ("error"). To fix this, standardize the status
field to use either HTTP status codes or string literals consistently throughout
the method. Additionally, add proper TypeScript type annotations to the method
signature and variables, specifying the expected types for
notificationDataArray, results, and the return type to improve type safety and
clarity.
1 participant
No description provided.