BAH-2180|sms-alerts-appointments-booking #89
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| package org.openmrs.module.appointments.notification; | ||
|
|
||
| import java.util.Date; | ||
|
|
||
| public interface SmsSender { | ||
| void send(String recipientMobileNumber, String patientName, String smsType, String serviceName, Date startDateTime, String teleLink); | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| package org.openmrs.module.appointments.notification.impl; | ||
|
|
||
| import org.apache.commons.lang3.StringUtils; | ||
| import org.apache.commons.logging.Log; | ||
| import org.apache.commons.logging.LogFactory; | ||
| import org.openmrs.Patient; | ||
| import org.openmrs.PersonAttribute; | ||
| import org.openmrs.api.context.Context; | ||
| import org.openmrs.module.appointments.model.Appointment; | ||
| import org.openmrs.module.appointments.notification.AppointmentEventNotifier; | ||
| import org.openmrs.module.appointments.notification.NotificationException; | ||
| import org.openmrs.module.appointments.notification.NotificationResult; | ||
| import org.openmrs.module.appointments.notification.SmsSender; | ||
|
|
||
| public class DefaultAppointmentPatientSmsNotifier implements AppointmentEventNotifier { | ||
| private Log log = LogFactory.getLog(this.getClass()); | ||
|
|
||
| private final static String PROP_SEND_APPT_SMS = "bahmni.appointment.sendSms"; | ||
|
|
||
| private static final String SMS_NOT_CONFIGURED = "SMS notification can not be sent to patient. Phone number not configured."; | ||
| private static final String SMS_SENT = "SMS sent to patient"; | ||
| private static final String MEDIUM_SMS = "SMS"; | ||
| private static final String SMS_FAILURE = "Failed to send sms to patient"; | ||
| private static final String SMS_NOT_SENT = "SMS notification not configured to be sent to patient."; | ||
|
|
||
| private SmsSender smsSender; | ||
|
|
||
| public DefaultAppointmentPatientSmsNotifier() {} | ||
|
|
||
| public DefaultAppointmentPatientSmsNotifier(SmsSender smsSender) { | ||
| this.smsSender = smsSender; | ||
| } | ||
|
|
||
| @Override | ||
| public String getMedium() { | ||
| return MEDIUM_SMS; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isApplicable(final Appointment appointment) { | ||
| boolean sendSmsToPatient = shouldSendSmsToPatient(); | ||
| if (!sendSmsToPatient) { | ||
| log.warn(SMS_NOT_SENT); | ||
| } | ||
| return sendSmsToPatient; | ||
| } | ||
|
|
||
| @Override | ||
| public NotificationResult sendNotification(final Appointment appointment) throws NotificationException { | ||
| Patient patient = appointment.getPatient(); | ||
| PersonAttribute patientPhoneAttribute = patient.getPerson().getAttribute("phoneNumber"); | ||
| if (patientPhoneAttribute != null) { | ||
| String patientPhoneNumber = patientPhoneAttribute.getValue(); | ||
| String patientName = appointment.getPatient().getGivenName(); | ||
| String apptServiceName = appointment.getService().getName(); | ||
| String teleLink = StringUtils.isNotBlank(appointment.getTeleHealthVideoLink()) ? appointment.getTeleHealthVideoLink() : ""; | ||
| String smsType = appointment.getAppointmentKind().toString(); | ||
| try { | ||
| log.info("Sending sms through: " + smsSender.getClass()); | ||
| smsSender.send(patientPhoneNumber, patientName, smsType, apptServiceName, appointment.getStartDateTime(), teleLink); | ||
| return new NotificationResult("", "SMS", 0, SMS_SENT); | ||
| } catch (Exception e) { | ||
| log.error(SMS_FAILURE, e); | ||
| throw new NotificationException(SMS_FAILURE, e); | ||
| } | ||
| } else { | ||
| log.warn(SMS_NOT_CONFIGURED); | ||
| return new NotificationResult(null, "SMS", 1, SMS_NOT_CONFIGURED); | ||
| } | ||
| } | ||
|
|
||
| private boolean shouldSendSmsToPatient() { | ||
| String shouldSendEmail = Context.getAdministrationService().getGlobalProperty(PROP_SEND_APPT_SMS, "false"); | ||
| return Boolean.valueOf(shouldSendEmail); | ||
| } | ||
|
|
||
| public void setSmsSender(SmsSender smsSender) { | ||
| log.warn("Replacing default SmsSender: " + this.smsSender + ", with:" + smsSender); | ||
| this.smsSender = smsSender; | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,180 @@ | ||
| package org.openmrs.module.appointments.notification.impl; | ||
|
|
||
| import org.apache.commons.lang.StringUtils; | ||
| import org.apache.commons.logging.Log; | ||
| import org.apache.commons.logging.LogFactory; | ||
| import org.openmrs.api.AdministrationService; | ||
| import org.openmrs.module.appointments.notification.SmsSender; | ||
| import org.openmrs.util.OpenmrsUtil; | ||
|
|
||
| import java.io.IOException; | ||
| import java.net.HttpURLConnection; | ||
| import java.net.URL; | ||
| import java.net.URLEncoder; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.nio.file.Paths; | ||
| import java.text.SimpleDateFormat; | ||
| import java.util.Date; | ||
| import java.util.Properties; | ||
|
|
||
| public class DefaultSmsSender implements SmsSender { | ||
|
|
||
| private Log log = LogFactory.getLog(this.getClass()); | ||
|
|
||
| private static final String SMS_PROPERTIES_FILENAME = "sms-config.properties"; | ||
| private static Properties smsSessionProperties = null; | ||
|
|
||
| SimpleDateFormat fmtTime = new SimpleDateFormat("HH:mm a"); | ||
| SimpleDateFormat fmtDateTele = new SimpleDateFormat("dd/MM/yyyy"); | ||
|
|
||
| private AdministrationService administrationService; | ||
|
|
||
| public DefaultSmsSender(AdministrationService administrationService) { | ||
| this.administrationService = administrationService; | ||
| } | ||
|
|
||
| @Override | ||
| public void send(String recipientMobileNumber, String patientName, String smsType, String serviceName, Date startDateTime, String teleLink) { | ||
| HttpURLConnection uc = null; | ||
| try { | ||
| getSession(); | ||
|
|
||
| String password = ""; | ||
| String addOnParams = ""; | ||
| String entityId = ""; | ||
| String templateId = ""; | ||
| String msgText = ""; | ||
|
|
||
| recipientMobileNumber = recipientMobileNumber.replace("+", ""); | ||
| int recipientPhoneLengthRequired = Integer.parseInt(smsSessionProperties.getProperty("sms.phonenumber.length")); | ||
| if (recipientMobileNumber.length()>recipientPhoneLengthRequired) { | ||
| int indexStart = recipientMobileNumber.length() - recipientPhoneLengthRequired; | ||
| recipientMobileNumber = recipientMobileNumber.substring(indexStart); | ||
| } | ||
| String dateSuffixText = getDateWithSuffix(startDateTime.getDate()); | ||
| String dateApptPattern = String.format("MMMM '%s', yyyy", dateSuffixText); | ||
| SimpleDateFormat fmtDateAppt = new SimpleDateFormat(dateApptPattern); | ||
|
|
||
| String requestUrl = smsSessionProperties.getProperty("sms.url"); | ||
|
There was a problem hiding this comment. A lot of logic for string concatenation. Needs tests. See the previous comment on how this could possibly be restructured into an AppointmentMessage class, which constructs the message string. |
||
| String apiKey = smsSessionProperties.getProperty("sms.key.username") + "=" + URLEncoder.encode(smsSessionProperties.getProperty("sms.username"), "UTF-8"); | ||
| String sender = "&" + smsSessionProperties.getProperty("sms.key.sender") + "=" + URLEncoder.encode(smsSessionProperties.getProperty("sms.sender"), "UTF-8"); | ||
| String number = "&" + smsSessionProperties.getProperty("sms.key.phonenumber") + "=" + URLEncoder.encode(recipientMobileNumber, "UTF-8"); | ||
|
|
||
| if (StringUtils.isNotBlank(smsSessionProperties.getProperty("sms.key.password"))) { | ||
| password = "&" + smsSessionProperties.getProperty("sms.key.password") + "=" + URLEncoder.encode(smsSessionProperties.getProperty("sms.password") , "UTF-8"); | ||
| } | ||
| if (StringUtils.isNotBlank(smsSessionProperties.getProperty("sms.key.entity.identifier"))) { | ||
| entityId = "&" + smsSessionProperties.getProperty("sms.key.entity.identifier") + "=" + URLEncoder.encode(smsSessionProperties.getProperty("sms.entity.identifier"), "UTF-8"); | ||
| } | ||
| if (StringUtils.isNotBlank(smsSessionProperties.getProperty("sms.key.template.identifier")) && "Virtual".equalsIgnoreCase(smsType)) { | ||
| templateId = "&" + smsSessionProperties.getProperty("sms.key.template.identifier") + "=" + | ||
| URLEncoder.encode(smsSessionProperties.getProperty("sms.appointment.teleconsultation.template.identifier"), "UTF-8"); | ||
| // Dear %s,\nTele-Consultation booked for %s %s\nVideo link: %s Bahmni Hospital(IPLit) | ||
|
|
||
| msgText = String.format(smsSessionProperties.getProperty("sms.appointment.teleconsultation.message.template"), patientName, | ||
| fmtDateTele.format(startDateTime), fmtTime.format(startDateTime), teleLink); | ||
| } | ||
| if (StringUtils.isNotBlank(smsSessionProperties.getProperty("sms.key.template.identifier")) && "WalkIn".equalsIgnoreCase(smsType)) { | ||
| templateId = "&" + smsSessionProperties.getProperty("sms.key.template.identifier") + "=" + | ||
| URLEncoder.encode(smsSessionProperties.getProperty("sms.appointment.walkin.template.identifier"), "UTF-8"); | ||
| // Dear %s,\n Your appointment is booked for %s on %s at %s by Bahmni Hospital (powered by IPLit) | ||
|
|
||
| msgText = String.format(smsSessionProperties.getProperty("sms.appointment.walkin.message.template"), patientName, | ||
| serviceName, fmtDateAppt.format(startDateTime), fmtTime.format(startDateTime)); | ||
| } | ||
| if (StringUtils.isNotBlank(smsSessionProperties.getProperty("sms.params.addons"))) { | ||
| addOnParams = "&" + smsSessionProperties.getProperty("sms.params.addons"); | ||
| } | ||
| String message = "&" + smsSessionProperties.getProperty("sms.key.message") + "=" + URLEncoder.encode(msgText, "UTF-8"); | ||
|
There was a problem hiding this comment. use a StringJoiner or a StringBuilder |
||
| requestUrl += apiKey + message + sender + number + password + entityId + templateId + addOnParams; | ||
|
There was a problem hiding this comment. sending the msg itself through the URL is not a standard way of passing the sms. Can you check if you can pass through the body/payload .. |
||
|
|
||
| URL url = new URL(requestUrl); | ||
| uc = (HttpURLConnection)url.openConnection(); | ||
| String smsResponse = uc.getResponseMessage(); | ||
|
There was a problem hiding this comment. if you aren't logging the response, why assign to a variable? |
||
| } catch (Exception e) { | ||
| throw new RuntimeException("Error occurred while sending sms", e); | ||
| } finally { | ||
| if (uc!=null) { | ||
| uc.disconnect(); | ||
| uc = null; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private Properties getSession() { | ||
|
There was a problem hiding this comment. why is this method called getSession()? all it does it load properties. |
||
| if (smsSessionProperties == null) { | ||
| Properties sessionProperties = smsSessionPropertiesFromPath(); | ||
| if (sessionProperties == null) { | ||
| log.warn("Could not load sms properties from application data directory file. Loading from OMRS settings."); | ||
| sessionProperties = smsSessionPropertiesFromOMRS(); | ||
| } | ||
| smsSessionProperties = sessionProperties; | ||
| } | ||
| return smsSessionProperties; | ||
| } | ||
|
|
||
| /** | ||
| * To be used as fallback. SMS properties are visible in openmrs settings. | ||
| * @param as | ||
| * @return | ||
| */ | ||
| private Properties smsSessionPropertiesFromOMRS() { | ||
| Properties p = new Properties(); | ||
| p.put("sms.key.username", administrationService.getGlobalProperty("sms.key.username", "")); | ||
| p.put("sms.key.password", administrationService.getGlobalProperty("sms.key.password", "")); | ||
| p.put("sms.key.phonenumber", administrationService.getGlobalProperty("sms.key.phonenumber", "")); | ||
| p.put("sms.key.message", administrationService.getGlobalProperty("sms.key.message", "")); | ||
| p.put("sms.key.sender", administrationService.getGlobalProperty("sms.key.sender", "")); | ||
| p.put("sms.key.entity.identifier", administrationService.getGlobalProperty("sms.key.entity.identifier", "")); | ||
| p.put("sms.key.template.identifier", administrationService.getGlobalProperty("sms.key.template.identifier", "")); | ||
|
|
||
| p.put("sms.url", administrationService.getGlobalProperty("sms.url", "")); | ||
| p.put("sms.username", administrationService.getGlobalProperty("sms.username", "")); | ||
| p.put("sms.password", administrationService.getGlobalProperty("sms.password", "")); | ||
| p.put("sms.sender", administrationService.getGlobalProperty("sms.sender", "")); | ||
| p.put("sms.entity.identifier", administrationService.getGlobalProperty("sms.entity.identifier", "")); | ||
| p.put("sms.appointment.walkin.template.identifier", administrationService.getGlobalProperty("sms.appointment.walkin.template.identifier", "")); | ||
| p.put("sms.appointment.teleconsultation.template.identifier", administrationService.getGlobalProperty("sms.appointment.teleconsultation.template.identifier", "")); | ||
| p.put("sms.appointment.walkin.message.template", administrationService.getGlobalProperty("sms.appointment.walkin.message.template", "")); | ||
| p.put("sms.appointment.teleconsultation.message.template", administrationService.getGlobalProperty("sms.appointment.teleconsultation.message.template", "")); | ||
| p.put("sms.params.addons", administrationService.getGlobalProperty("sms.params.addons", "")); | ||
| return p; | ||
| } | ||
|
|
||
| private Properties smsSessionPropertiesFromPath() { | ||
| Path propertyFilePath = Paths.get(OpenmrsUtil.getApplicationDataDirectory(), SMS_PROPERTIES_FILENAME); | ||
| if (Files.exists(propertyFilePath)) { | ||
| Properties properties = new Properties(); | ||
| try { | ||
| log.info("Reading properties from: " + propertyFilePath); | ||
| properties.load(Files.newInputStream(propertyFilePath)); | ||
| return properties; | ||
| } catch (IOException e) { | ||
| log.error("Could not load sms properties from: " + propertyFilePath, e); | ||
| } | ||
| } else { | ||
| log.warn("No sms configuration defined at " + propertyFilePath); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| private String getDateWithSuffix(int date) { | ||
|
There was a problem hiding this comment. do we really need to hardcode st, nd etc .. why not just convert to string representation of date .. like 30 June 2022. its also not using the locale and using just english. You can leverage a date formatter There was a problem hiding this comment. sure. Will incorporate all the comments. |
||
| switch (date) { | ||
| case 1: | ||
| case 21: | ||
| case 31: | ||
| return "" + date + "st"; | ||
|
|
||
| case 2: | ||
| case 22: | ||
| return "" + date + "nd"; | ||
|
|
||
| case 3: | ||
| case 23: | ||
| return "" + date + "rd"; | ||
|
|
||
| default: | ||
| return "" + date + "th"; | ||
| } | ||
| } | ||
|
|
||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I was hoping the interface is Channel vocabulary based and not Appointment vocabulary based. Something like:
MessageSender.send(String phoneNumber, String fullMessage).
Then SMS Sender, Whatsapp Sender, etc can easily implement this interface. They don't care if the message is goign to patient, doctor, customer, relative, etc. They just send a message over the appropriate channel to the number. Its the caller of these classes that constructs the message and delegates the "sending" to the channel implementation.
So.. Appointment ---delegates to ---> AppointmentMessage class ---delegates to--> SMSSender class ---> sends SMS
Similarly,
Appointment ---delegates to ---> AppointmentMessage class ---delegates to (based on config)--> WhatsappSender class ---> sends SMS
Then we write unit tests for AppointmentMessage class which checks if the construction of message is correct based on passed parameters like PatientName, Service Name, Date, Link.
With this logic, the MessageSender interface and its implementations are reusable in the context of different messages (Appointments, Registration, Teleconsult, Doctor-Doctor consult, etc).
For this JIRA ticket, we would only implement SMS (and not whatzapp classes).
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.
Building of the message text requires sms template identifier. The message template format is available in SMSSender class (DefaultSmsSender) which is needed as outgoing SMS has to be conforming to this format.
Either message template is available in config file or in global properties in openmrs.
Please advise
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.
Maybe we don't need different templates for SMS or Whatzapp. We just create a template called: appointmentMessageTemplate and reuse the same template for SMS or Whatzapp etc... Or, depending on the current configured settings (SMS, Whatsapp, etc) -- the loaded properties file by AppointmentMessage class is: load appointmentMessageTemplate-sms.properties and constructs the string message, and calls the SMSSender.