feat(providers): add text.lk sms provider integration#9975
feat(providers): add text.lk sms provider integration#9975ritambh888 wants to merge 10 commits intonovuhq:nextfrom
Conversation
👷 Deploy request for dashboard-v2-novu-staging pending review.Visit the deploys page to approve it
|
WalkthroughAdds an exported TextLkSmsProvider class implementing ISmsProvider with id 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/providers/src/lib/sms/textlk/textlk.provider.ts`:
- Around line 24-29: The payload currently hard-codes sender_id to 'Text.lk'
which prevents per-message sender customization; update the payload construction
in textlk.provider.ts (where payload is built) to use options.from if present
(falling back to 'Text.lk' as default) so the sender_id supports
ISmsOptions.from values (ensure it accepts alphanumeric up to 11 chars or
phone-format as the API expects) and preserve recipient/message fields as-is.
🧹 Nitpick comments (2)
packages/providers/src/lib/sms/textlk/textlk.provider.spec.ts (1)
4-5: Tighten assertions and drop trivial comments.Capture the response to validate mapping, and remove inline comments that don’t add value.
🧪 Suggested update
-// Mock axios so we don't actually send SMS during tests const mockPost = vi.fn(); @@ - await provider.sendMessage({ + const result = await provider.sendMessage({ to: '+94771234567', content: 'Hello World', }); - // Check if axios.post was called expect(mockPost).toHaveBeenCalled(); - // Check if called with the right URL and Payload expect(mockPost).toHaveBeenCalledWith( 'https://app.text.lk/api/v3/sms/send', { recipient: '+94771234567', sender_id: 'Text.lk', type: 'plain', message: 'Hello World', }, { headers: { 'Authorization': 'Bearer test-api-key', 'Content-Type': 'application/json', 'Accept': 'application/json', }, } ); + + expect(result).toEqual({ + id: 'mock-uid-123', + date: expect.any(String), + });As per coding guidelines, don't leave comments in code, unless they explain something complex and not trivial.
Also applies to: 27-35
packages/providers/src/lib/sms/textlk/textlk.provider.ts (1)
9-17: Refactor to factory function + interface to align with TypeScript style guidelines; note this pattern is used across all SMS providers.The class-based pattern violates the guideline to use functional and declarative programming patterns. While a factory function refactor is appropriate, note that all ~35 SMS providers in this directory currently use the same class-based structure, so this change should either be applied consistently across all providers or deferred as a broader refactoring initiative.
Additionally, remove the unnecessary comment on line 26 (
// Default sender. This can be made configurable later if requested.). It explains something trivial and clutters the code.
ritambh888
force-pushed
the
feature/add-textlk-provider
branch
from
6151266 to
712c94a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/providers/src/lib/sms/textlk/textlk.provider.ts`:
- Around line 24-29: The payload currently uses options.to verbatim; update the
recipient normalization before building payload so Text.lk gets numbers without
leading '+' and handles comma-separated lists. In the function that constructs
payload (where payload and options.to are used), split options.to on commas,
trim each entry, strip any leading '+' from each phone number, then rejoin (or
pass the array) into the recipient field so the API receives numbers in the
94XXXXXXXXX format; ensure this logic is applied where payload is created
(referencing payload and options.to).
🧹 Nitpick comments (4)
packages/providers/src/lib/sms/textlk/textlk.provider.ts (3)
1-1: Consider dash-separated filename.The filename uses a dot separator; please align with the dash-based naming convention (e.g.,
textlk-provider.ts) and update imports accordingly. As per coding guidelines, Use lowercase with dashes for directories and files (e.g., components/auth-wizard).
9-17: Prefer a functional provider factory (if the SMS factory allows it).Guidelines favor functional/declarative patterns over classes; if the provider registry doesn’t require instantiation, consider exporting a factory function that returns an
ISmsProvider. As per coding guidelines, Use functional and declarative programming patterns; avoid classes.♻️ Suggested refactor (if compatible)
-export class TextLkSmsProvider implements ISmsProvider { - id = 'textlk'; - channelType = ChannelTypeEnum.SMS as ChannelTypeEnum.SMS; - - constructor( - private config: { - apiKey: string; - } - ) {} - - async sendMessage( - options: ISmsOptions - ): Promise<ISendMessageSuccessResponse> { - const BASE_URL = 'https://app.text.lk/api/v3/sms/send'; - - const payload = { - recipient: options.to, - sender_id: options.from || 'Text.lk', - type: 'plain', - message: options.content, - }; - - const response = await axios.post(BASE_URL, payload, { - headers: { - 'Authorization': `Bearer ${this.config.apiKey}`, - 'Content-Type': 'application/json', - 'Accept': 'application/json', - }, - }); - - return { - id: response.data.uid || new Date().getTime().toString(), - date: new Date().toISOString(), - }; - } -} +export function createTextLkSmsProvider( + config: { apiKey: string } +): ISmsProvider { + async function sendMessage( + options: ISmsOptions + ): Promise<ISendMessageSuccessResponse> { + const BASE_URL = 'https://app.text.lk/api/v3/sms/send'; + + const payload = { + recipient: options.to, + sender_id: options.from || 'Text.lk', + type: 'plain', + message: options.content, + }; + + const response = await axios.post(BASE_URL, payload, { + headers: { + 'Authorization': `Bearer ${config.apiKey}`, + 'Content-Type': 'application/json', + 'Accept': 'application/json', + }, + }); + + return { + id: response.data.uid || new Date().getTime().toString(), + date: new Date().toISOString(), + }; + } + + return { + id: 'textlk', + channelType: ChannelTypeEnum.SMS as ChannelTypeEnum.SMS, + sendMessage, + }; +}
13-16: Extract the config shape into an interface.This keeps the contract reusable and aligns with the “prefer interfaces” rule. As per coding guidelines, Prefer interfaces over types in TypeScript.
♻️ Suggested change
+interface TextLkSmsProviderConfig { + apiKey: string; +} + export class TextLkSmsProvider implements ISmsProvider { @@ - constructor( - private config: { - apiKey: string; - } - ) {} + constructor(private config: TextLkSmsProviderConfig) {}packages/providers/src/lib/sms/textlk/textlk.provider.spec.ts (1)
1-1: Consider dash-separated spec filename.Rename to
textlk-provider.spec.tsto align with the dash-based naming rule and update imports accordingly. As per coding guidelines, Use lowercase with dashes for directories and files (e.g., components/auth-wizard).
| const payload = { | ||
| recipient: options.to, | ||
| sender_id: options.from || 'Text.lk', | ||
| type: 'plain', | ||
| message: options.content, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Text.lk SMS API recipient format documentation E.164
💡 Result:
Text.lk’s SMS API recipient is not strict E.164 with a leading +. Their docs show it as country code + number, digits only:
- Format:
94XXXXXXXXX(Sri Lanka example:94712345678) — no leading0, no+. [1][2] - Multiple recipients: comma-separated, e.g.
9476000000,94710000000(as documented). [1]
So if you have an E.164 number like +94712345678, send it as 94712345678. [2]
Sources:
[1] Text.lk “Send SMS” API docs (recipient examples and comma-separated format)
[2] Text.lk PHP integration guide (“Phone Number Format” correct/incorrect examples)
🏁 Script executed:
# Search for Text.lk provider tests and documentation
find . -type f -name "*textlk*" | head -20Repository: novuhq/novu
Length of output: 177
🏁 Script executed:
# Search for any documentation or comments about recipient format in the codebase
rg -i "textlk|text\.lk" -A 5 -B 5 | head -100Repository: novuhq/novu
Length of output: 4929
Normalize recipient phone number for Text.lk API.
Text.lk API requires phone numbers without the leading + prefix in the format 94XXXXXXXXX (country code + digits only). The current implementation passes options.to verbatim; if a +-prefixed E.164 number is provided (as shown in tests), the API will reject it. Remove the + prefix and handle comma-separated numbers if present.
Normalization example
- const payload = {
- recipient: options.to,
+ const recipient = options.to
+ .split(',')
+ .map((value) => value.trim().replace(/^\+/, ''))
+ .join(',');
+
+ const payload = {
+ recipient,
sender_id: options.from || 'Text.lk',
type: 'plain',
message: options.content,
};📝 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 payload = { | |
| recipient: options.to, | |
| sender_id: options.from || 'Text.lk', | |
| type: 'plain', | |
| message: options.content, | |
| }; | |
| const recipient = options.to | |
| .split(',') | |
| .map((value) => value.trim().replace(/^\+/, '')) | |
| .join(','); | |
| const payload = { | |
| recipient, | |
| sender_id: options.from || 'Text.lk', | |
| type: 'plain', | |
| message: options.content, | |
| }; |
🤖 Prompt for AI Agents
In `@packages/providers/src/lib/sms/textlk/textlk.provider.ts` around lines 24 -
29, The payload currently uses options.to verbatim; update the recipient
normalization before building payload so Text.lk gets numbers without leading
'+' and handles comma-separated lists. In the function that constructs payload
(where payload and options.to are used), split options.to on commas, trim each
entry, strip any leading '+' from each phone number, then rejoin (or pass the
array) into the recipient field so the API receives numbers in the 94XXXXXXXXX
format; ensure this logic is applied where payload is created (referencing
payload and options.to).
8de565e to
0200d1d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/providers/src/lib/sms/textlk/textlk.provider.spec.ts`:
- Around line 37-48: The axios.post call (BASE_URL, payload) should be wrapped
in a try-catch to handle network/timeouts and API errors; call axios.post with a
timeout option (e.g., { timeout: ... , headers: {...} }) to avoid hanging,
validate response and response.data before reading response.data.uid (fall back
to a deterministic id if missing), and in the catch block throw or return a
clear, contextual error message that includes the original error and the request
context (BASE_URL/payload) so callers can handle failures gracefully.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Description
Implemented the Text.lk SMS provider integration as requested in issue #9725.
Changes
TextLkSmsProviderimplementation usingaxios.vitestmocking.Related Issue
Closes #9725