Update Africa's Talking Live API Endpoint

[Pinchez25]

No description provided.

[willymwai]

/describe

[willymwai]

/review

[willymwai]

/improve

[qodo-code-review]

Title

Update Africa's Talking Live API Endpoint

---

PR Type

Enhancement, Bug fix

---

Description

• Refactored Africa's Talking SMS provider to separate sandbox and live logic

  • Introduced sendMessageSandbox and sendMessageLive methods
  • Improved error handling and logging for API responses
  • Enforced senderId requirement for live environment

• Updated SMS delivery status mapping for Africa's Talking

  • Status code 100 now maps to SENT instead of PENDING

• Enhanced SMSBridge configuration handling

  • Ensured bridge configs are cleared and associated correctly
  • Each config is now linked to its SMSBridge instance

• Added GitHub workflow for automated PR review comments

---

Changes walkthrough 📝

	Relevant files
Enhancement	AfricastalkingMessageProvider.java Refactor and enhance Africa's Talking SMS sending logic    src/main/java/org/fineract/messagegateway/sms/providers/impl/africastalking/AfricastalkingMessageProvider.java Refactored to separate sandbox and live SMS sending logic Added sendMessageSandbox, sendMessageLive, and processResponse methods Improved logging and error handling for API requests and responses Enforced senderId requirement for live environment +90/-61  SMSBridge.java Improve SMSBridge config association and management            src/main/java/org/fineract/messagegateway/sms/domain/SMSBridge.java Updated setSmsBridgeConfig to clear and re-add configs Ensured each config is associated with the current SMSBridge +8/-1      SmsBridgeSerializer.java Ensure configs are linked to their SMSBridge                          src/main/java/org/fineract/messagegateway/sms/serialization/SmsBridgeSerializer.java Set SMSBridge reference on each config during assembly +1/-0
Bug fix	AfricastalkingStatus.java Update Africa's Talking status code mapping                            src/main/java/org/fineract/messagegateway/sms/providers/impl/africastalking/AfricastalkingStatus.java Changed status mapping for code 100 from PENDING to SENT +1/-1
Configuration changes	qodo-review-comment.yml Add workflow for automated PR review comments                        .github/workflows/qodo-review-comment.yml Added GitHub Actions workflow to automate PR review comments +74/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 Security concerns Sensitive information exposure: The code logs API keys and message content in the sendMessageSandbox method (lines 60-64). This could expose sensitive information in log files. Consider masking or removing sensitive data like API keys and message content from logs.

⚡ Recommended focus areas for review Potential NPE The processResponse method doesn't check if recipients array is null before accessing it. It only checks if it's empty, which could lead to a NullPointerException if the SMSMessageData doesn't contain a Recipients array. if (!recipients.isEmpty()) { JSONObject recipient = recipients.getJSONObject(0); // Update the message with external ID and delivery status String messageId = recipient.getString("messageId"); int statusCode = recipient.getInt("statusCode"); logger.info("Africa's Talking API response - MessageId: {}, StatusCode: {}", messageId, statusCode); SmsMessageStatusType deliveryStatus = AfricastalkingStatus.smsStatus(statusCode); logger.info("Mapped delivery status: {}", deliveryStatus); message.setExternalId(messageId); message.setDeliveryStatus(deliveryStatus.getValue()); } else { String errorMessage = smsMessageData.getString("Message"); logger.error("Failed to send SMS. Error message: {}", errorMessage); throw new MessageGatewayException("Failed to send SMS. Error message: " + errorMessage); } Condition Syntax The GitHub workflow has a condition using ${{ env.MAIN_BRANCH }} in an if statement, but this syntax is incorrect for direct comparison with github.base_ref. This will likely cause the workflow to fail. if: github.event.action == 'opened' && github.base_ref == ${{ env.MAIN_BRANCH }} uses: actions/github-script@v7 Redundant Code The setSMSBridgeToBridgeConfigs method and the new setSmsBridgeConfig method have overlapping functionality. Both set the SMSBridge on config objects, which could lead to confusion about which method should be used. public void setSMSBridgeToBridgeConfigs() { if (this.bridgeConfigurations != null) { for (SMSBridgeConfig config : this.bridgeConfigurations) { config.setSMSBridge(this); } } } public void setSmsBridgeConfig(final Collection<SMSBridgeConfig> bridgeConfigurations) { this.bridgeConfigurations.clear(); // Add new configurations if (bridgeConfigurations != null) { for (SMSBridgeConfig config : bridgeConfigurations) { config.setSMSBridge(this); this.bridgeConfigurations.add(config); } }

[qodo-code-review]

PR Code Suggestions ✨

Category	Suggestion	Impact
Possible issue	✅ Fix JSONArray method call Suggestion Impact: The commit implemented exactly what was suggested - replacing the isEmpty() method (which doesn't exist on JSONArray) with length() > 0 to properly check if the array has elements code diff: - if (!recipients.isEmpty()) { + + if (recipients.length() > 0) { The isEmpty() method is not available on JSONArray. Use recipients.length() > 0 instead to check if the array has elements, which matches the original implementation and prevents a runtime error. src/main/java/org/fineract/messagegateway/sms/providers/impl/africastalking/AfricastalkingMessageProvider.java [102] -if (!recipients.isEmpty()) { +if (recipients.length() > 0) { [Suggestion processed] Suggestion importance[1-10]: 10 __ Why: This is a critical bug fix. The code uses isEmpty() which doesn't exist on JSONArray, which would cause a runtime error. Using length() > 0 is the correct way to check if a JSONArray has elements.	High
	Fix workflow condition syntax The comparison with ${{ env.MAIN_BRANCH }} is incorrect syntax in this context. Use string comparison with the environment variable directly to avoid workflow execution errors. .github/workflows/qodo-review-comment.yml [41] -if: github.event.action == 'opened' && github.base_ref == ${{ env.MAIN_BRANCH }} +if: github.event.action == 'opened' && github.base_ref == env.MAIN_BRANCH Apply / Chat Suggestion importance[1-10]: 9 __ Why: The workflow uses incorrect syntax for environment variable comparison which would cause the GitHub Action to fail. The suggested fix properly references env.MAIN_BRANCH without the unnecessary ${{ }} syntax in this context.	High
Update