merge dev changes to main branch for ISACC release (#63)

* Added the cli command for a single parameter to update for all patients, make sure phone numbers would be reused

* Added an inactive patient to test

* Added an inactive patient to test

* Cache Unchanged Layers (#50)

* Fixup default branch

* Prevent build version from invalidating cached pip installs

* Reuse unchanged image layers when building on CI

* fix malformed html in notification email (#54)

* fix malformed html in notification email

* fix based on feedback

---------

Co-authored-by: Amy Chen <[email protected]>

* WIP: modifying cli commands

* WIP: localizing the error

* WIP: fixing the cli error

* Fixed CLI

* WIP: fixing CLI

* WIP: fixing cli command

* WIP: reinstating abilities

* Adding email headers {To, Date, Message-Id} to further decrease spam (#55)

* Adding email headers {To, Date, Message-Id} to further decrease spam
score.

* Correct "EnvelopeFrom" address to match "From" header.

* name kwargs for clarity, as per code review suggestion.

* cli command to deactivate patient

* WIP: adding CLI to deactivate patient

* WIP: fixing deactivation CLI

* WIP: fixing deactivation CLI

* WIP: fixing CLI

* WIP: Fixing CLI

* WIP: fixing CLI

* WIP: changing active from bool to str

* WIP: experimenting with search values

* WIP: fixing boolean search

* WIP: fixing bool active

* WIP: fixing bool

* WIP: fixing active

* WIP: fixing active bool

* WIP: cleaning up the syntax

* WIP: experimenting

* WIP: fixed

* Removing unncessary methods

* Reinstated ability to deactivate a patient without active field

* Remove the trailing whitespace

* Addressing PR comments

* Fixing get_active_user

* Changing active check to be part of the query

* Formatting

* Raises a value error for inactive patient

* Simple fix to add a log whenever message fails

* WIP: Added a CLI to add start period for all patients without one

* Add CLI to add start to all entries

* Mark patient as active when 'start' included in the message

* Correspond to Twilio's opt in word.

* Use consistent format

* Stop deactivating messages that the user may need

* Fix the logic

* Move calculations to shared timezone

* Handling a case when phone number got removed from a patient

* Handle subsubcribed error separately

* Adding validating function for twilio

* Returns 200 when unsubscribed in nonactive patient messages the sms twilio phone number

* MessageStatus change to 200

* Added error logging

* Altered all 500 codes to 200

* Introducing different CR statuses, according to the type of error.

* Refactoring record creator methods into respective classes

* changing a name of the function

* WIP: communications refactoring work

* Adding logs

* Handling communications for unsubscribed patients

* Finished refactoring

* Disable the migration needed to update patient extensions, as it'll undo (#57)

user actions with https://www.pivotaltracker.com/story/show/186783090

* Removing CR logic, represent state in the Communication

* Fixing status appending

* Refactoring

* Refactoring to keep the communication creation in the single function

* Worked: outgoing messages

* Fixing record

* Modified logs

* Fixing status

---------

Co-authored-by: Daniil <[email protected]>
Co-authored-by: Ivan Cvitkovic <[email protected]>
Co-authored-by: Amy Chen <[email protected]>
Co-authored-by: Amy Chen <[email protected]>

.github/workflows/build-deliver.yaml

@@ -19,7 +19,7 @@ jobs:
 
       - name: Publish to GitHub Container Registry
         # TODO: pin to hash
-        uses: elgohr/Publish-Docker-Github-Action@master
+        uses: elgohr/Publish-Docker-Github-Action@main
         with:
           name: ${{ github.repository }}
           registry: ghcr.io
@@ -33,3 +33,5 @@ jobs:
           # create docker image tags to match git tags
           tag_names: true
           buildargs: VERSION_STRING
+          # cache layers that have not changed between builds (eg dependencies)
+          cache: true

Dockerfile

@@ -2,14 +2,14 @@ FROM python:3.7
 
 WORKDIR /opt/app
 
-ARG VERSION_STRING
-ENV VERSION_STRING=$VERSION_STRING
-
 COPY requirements.txt .
 RUN pip install --requirement requirements.txt
 
 COPY . .
 
+ARG VERSION_STRING
+ENV VERSION_STRING=$VERSION_STRING
+
 ENV FLASK_APP=isacc_messaging.app:create_app() \
     PORT=8000
 

isacc_messaging/api/email_notifications.py

@@ -46,8 +46,8 @@ def send_message_received_notification(recipients: list, patient: Patient):
         link_url=link_url,
         link_suffix_text="to view it",
         post_link_msg=(
-            f'<h3><a href="mailto:{SUPPORT_EMAIL}">Send Email</a>'
-            'if you have questions.</h3>'),
+            "If you are not the person who should be getting these messages, contact "
+            f'<a href="mailto:{SUPPORT_EMAIL}">your site lead</a>.'),
         unsubscribe_link=UNSUB_LINK)
 
     send_email(

isacc_messaging/api/views.py

@@ -1,10 +1,13 @@
+from functools import wraps
 import click
 import logging
-
+from datetime import datetime
 from flask import Blueprint, jsonify, request
+from flask import current_app
 
 from isacc_messaging.api.isacc_record_creator import IsaccRecordCreator
 from isacc_messaging.audit import audit_entry
+from twilio.request_validator import RequestValidator
 
 base_blueprint = Blueprint('base', __name__, cli_group=None)
 
@@ -94,13 +97,42 @@ def message_status_update():
     )
 
     record_creator = IsaccRecordCreator()
-    result = record_creator.on_twilio_message_status_update(request.values)
-    if result is not None:
-        return result, 500
+    try:
+        record_creator.on_twilio_message_status_update(request.values)
+    except Exception as ex:
+        audit_entry(
+            f"on_twilio_message_status_update generated error {ex}",
+            level='error'
+        )
+        return ex, 200
     return '', 204
 
 
+def validate_twilio_request(f):
+    """Validates that incoming requests genuinely originated from Twilio"""
+    @wraps(f)
+    def twilio_validation_decorated(*args, **kwargs):
+        validator = RequestValidator(current_app.config.get('TWILIO_AUTH_TOKEN'))
+        # Validate the request is from Twilio using its
+        # URL, POST data, and X-TWILIO-SIGNATURE header
+        request_valid = validator.validate(
+            request.url,
+            request.form,
+            request.headers.get('X-TWILIO-SIGNATURE', ''))
+
+        if not request_valid:
+            audit_entry(
+                f"sms request not from Twilio",
+                extra={'request.values': dict(request.values)},
+                level='error'
+            )
+            return '', 403
+        return f(*args, **kwargs)
+    return twilio_validation_decorated
+
+
 @base_blueprint.route("/sms", methods=['GET','POST'])
+@validate_twilio_request
 def incoming_sms():
     audit_entry(
         f"Call to /sms webhook",
@@ -111,6 +143,7 @@ def incoming_sms():
         record_creator = IsaccRecordCreator()
         result = record_creator.on_twilio_message_received(request.values)
     except Exception as e:
+        # Unexpected error
         import traceback, sys
         exc = sys.exc_info()[0]
         stack = traceback.extract_stack()
@@ -121,16 +154,20 @@ def incoming_sms():
         audit_entry(
             f"on_twilio_message_received generated: {stackstr}",
             level="error")
-        return stackstr, 500
+        return stackstr, 200
     if result is not None:
+        # Occurs when message is incoming from unknown phone 
+        # or request is coming from a subscribed phone number, but 
+        # internal logic renders it invalid
         audit_entry(
             f"on_twilio_message_received generated error {result}",
             level='error')
-        return result, 500
+        return result, 200
     return '', 204
 
 
 @base_blueprint.route("/sms-handler", methods=['GET','POST'])
+@validate_twilio_request
 def incoming_sms_handler():
     audit_entry(
         f"Received call to /sms-handler webhook (not desired)",
@@ -195,10 +232,73 @@ def send_system_emails(category, dry_run, include_test_patients):
 @click.option("--dry-run", is_flag=True, default=False, help="Simulate execution; don't persist to FHIR store")
 def update_patient_extensions(dry_run):
     """Iterate through active patients, update any stale/missing extensions"""
+    # this was a 1 and done migration method.  disable for now
+    raise click.ClickException(
+        "DISABLED: unsafe to run as this will now undo any user marked "
+        "read messages via "
+        "https://github.com/uwcirg/isacc-messaging-client-sof/pull/85")
     from isacc_messaging.models.fhir import next_in_bundle
     from isacc_messaging.models.isacc_patient import IsaccPatient as Patient
     active_patients = Patient.active_patients()
     for json_patient in next_in_bundle(active_patients):
         patient = Patient(json_patient)
         patient.mark_next_outgoing(persist_on_change=not dry_run)
         patient.mark_followup_extension(persist_on_change=not dry_run)
+
+
+@base_blueprint.cli.command("maintenance-reinstate-all-patients")
+def update_patient_active():
+    """Iterate through all patients, activate all of them"""
+    from isacc_messaging.models.fhir import next_in_bundle
+    from isacc_messaging.models.isacc_patient import IsaccPatient as Patient
+    all_patients = Patient.all_patients()
+    for json_patient in next_in_bundle(all_patients):
+        patient = Patient(json_patient)
+        patient.active = True
+        patient.persist()
+        audit_entry(
+            f"Patient {patient.id} active set to true",
+            level='info'
+        )
+
+
+@base_blueprint.cli.command("maintenance-add-telecom-period-start-all-patients")
+def update_patient_telecom():
+    """Iterate through patients, add telecom start period to all of them"""
+    from isacc_messaging.models.fhir import next_in_bundle
+    from isacc_messaging.models.isacc_patient import IsaccPatient as Patient
+    import fhirclient.models.period as period
+    from isacc_messaging.models.isacc_fhirdate import IsaccFHIRDate as FHIRDate
+    patients_without_telecom_period = Patient.all_patients()
+    new_period = period.Period()
+    # Get the current time in UTC
+    current_time = datetime.utcnow()
+    # Format the current time as per the required format
+    formatted_time = current_time.strftime('%Y-%m-%dT%H:%M:%SZ')
+    new_period.start = FHIRDate(formatted_time)
+    for json_patient in next_in_bundle(patients_without_telecom_period):
+        patient = Patient(json_patient)
+        if patient.telecom:
+            for telecom_entry in patient.telecom:
+                if telecom_entry.system.lower() == "sms" and not telecom_entry.period:
+                    telecom_entry.period = new_period
+                    patient.persist()
+                    audit_entry(
+                        f"Patient {patient.id} active telecom period set to start now",
+                        level='info'
+                    )
+
+
+@base_blueprint.cli.command("deactivate_patient")
+@click.argument('patient_id')
+def deactivate_patient(patient_id):
+    """Set the active parameter to false based on provided patient id"""
+    from isacc_messaging.models.isacc_patient import IsaccPatient as Patient
+    json_patient = Patient.get_patient_by_id(patient_id)
+    patient = Patient(json_patient)
+    patient.active = False
+    patient.persist()
+    audit_entry(
+        f"Patient {patient_id} active set to false",
+        level='info'
+    )

isacc_messaging/models/email.py

@@ -1,4 +1,5 @@
 """Module for email utility functions"""
+from email import utils
 from email.mime.multipart import MIMEMultipart
 from email.mime.text import MIMEText
 from flask import current_app
@@ -19,7 +20,10 @@ def send_email(recipient_emails: list, subject, text, html):
     msg = MIMEMultipart('alternative')
     msg['Subject'] = subject
     msg['From'] = sender_name
+    msg.add_header("To", ' '.join(recipient_emails))
     msg.add_header("List-Unsubscribe", f"{current_app.config.get('ISACC_APP_URL')}/unsubscribe")
+    msg.add_header("Date", utils.format_datetime(utils.localtime()))
+    msg.add_header("Message-Id", utils.make_msgid())
 
     # Record the MIME types of both parts - text/plain and text/html.
     part1 = MIMEText(text, 'plain')
@@ -36,8 +40,8 @@ def send_email(recipient_emails: list, subject, text, html):
 
     try:
         with smtplib.SMTP_SSL(email_server, port, context=context) as server:
-            server.login(sender_email, app_password)
-            server.sendmail(sender_email, recipient_emails, msg.as_string())
+            server.login(user=sender_email, password=app_password)
+            server.sendmail(from_addr=sender_name, to_addrs=recipient_emails, msg=msg.as_string())
             audit_entry(
                 f"Email notification sent",
                 extra={
@@ -54,3 +58,5 @@ def send_email(recipient_emails: list, subject, text, html):
                 'exception': str(e)},
             level='error'
         )
+        # present stack for easier debugging
+        raise e

isacc_messaging/models/isacc_communication.py

@@ -24,6 +24,17 @@ def is_manual_follow_up_message(self) -> bool:
                     if coding.code == 'isacc-manually-sent-message':
                         return True
 
+    def persist(self):
+        """Persist self state to FHIR store"""
+        response = HAPI_request('PUT', 'Communication', resource_id=self.id, resource=self.as_json())
+        return response
+
+    def change_status(self, status):
+        """Persist self state to FHIR store"""
+        self.status = status
+        response = self.persist()
+        return response
+
     @staticmethod
     def about_patient(patient):
         """Query for "outside" Communications about the patient

isacc_messaging/models/isacc_communicationrequest.py

@@ -3,7 +3,10 @@
 Captures common methods needed by ISACC for CommunicationRequests, by specializing
 the `fhirclient.CommunicationRequest` class.
 """
+from datetime import datetime
 from fhirclient.models.communicationrequest import CommunicationRequest
+from fhirclient.models.identifier import Identifier
+from isacc_messaging.audit import audit_entry
 
 from isacc_messaging.models.fhir import HAPI_request, first_in_bundle
 
@@ -29,3 +32,83 @@ def next_by_patient(patient):
         first = first_in_bundle(response)
         if first:
             return CommunicationRequest(first)
+
+    def dispatched(self):
+        return self.identifier and len([i for i in self.identifier if i.system == "http://isacc.app/twilio-message-sid"]) > 0
+
+    def dispatched_message_status(self):
+            sid = ""
+            status = ""
+            as_of = ""
+            for i in self.identifier:
+                for e in i.extension:
+                    if e.url == "http://isacc.app/twilio-message-status":
+                        status = e.valueCode
+                    if e.url == "http://isacc.app/twilio-message-status-updated":
+                        as_of = e.valueDateTime.isostring
+                if i.system == "http://isacc.app/twilio-message-sid":
+                    sid = i.value
+            return f"Twilio message (sid: {sid}, CR.id: {self.id}) was previously dispatched. Last known status: {status} (as of {as_of})"
+
+    def mark_dispatched(self, expanded_payload, result):
+            self.payload[0].contentString = expanded_payload
+            if not self.identifier:
+                self.identifier = []
+            self.identifier.append(Identifier({
+                "system": "http://isacc.app/twilio-message-sid",
+                "value": result.sid,
+                "extension": [
+                    {
+                        "url": "http://isacc.app/twilio-message-status",
+                        "valueCode": result.status
+                    },
+                    {
+                        "url": "http://isacc.app/twilio-message-status-updated",
+                        "valueDateTime": datetime.now().astimezone().isoformat()
+                    },
+                ]
+            }))
+            updated_cr = HAPI_request('PUT', 'CommunicationRequest', resource_id=self.id, resource=self.as_json())
+            return updated_cr
+
+    def create_communication_from_request(self, status = "completed"):
+        if self.category[0].coding[0].code == 'isacc-manually-sent-message':
+            code = 'isacc-manually-sent-message'
+        else:
+            code = "isacc-auto-sent-message"
+        return {
+            "resourceType": "Communication",
+            "id": str(self.id),
+            "basedOn": [{"reference": f"CommunicationRequest/{self.id}"}],
+            "partOf": [{"reference": f"{self.basedOn[0].reference}"}],
+            "category": [{
+                "coding": [{
+                    "system": "https://isacc.app/CodeSystem/communication-type",
+                    "code": code
+                }]
+            }],
+
+            "payload": [p.as_json() for p in self.payload],
+            "sent": datetime.now().astimezone().isoformat(),
+            "sender": self.sender.as_json() if self.sender else None,
+            "recipient": [r.as_json() for r in self.recipient],
+            "medium": [{
+                "coding": [{
+                    "system": "http://terminology.hl7.org/ValueSet/v3-ParticipationMode",
+                    "code": "SMSWRIT"
+                }]
+            }],
+            "note": [n.as_json() for n in self.note] if self.note else None,
+            "status": status
+        }
+
+    def persist(self):
+        """Persist self state to FHIR store"""
+        response = HAPI_request('PUT', 'CommunicationRequest', resource_id=self.id, resource=self.as_json())
+        return response
+
+    def report_cr_status(self, status_reason):
+        audit_entry(
+            f"CommunicationRequest({self.id}) status set to {self.status} because {status_reason}",
+            extra={"CommunicationRequest": self.as_json(), "reason": status_reason}
+        )