stop emailing withdrawn (#4373)

Discovered withdrawn users continue to be sent reminder emails.

- This PR patches a number of incorrect usages of `user.email_ready()`
(as it returns a tuple, a simple `if user.email_ready()` check is
misleading.
- masks the email address of all withdrawn users, to make certain they
aren't sent any additional messages (via a migration)
- blocks another path to hand trigger sending reminders. I doubt this
was being used, but no reason to leave door open

portal/migrations/versions/5caf794c70a7_.py

@@ -0,0 +1,152 @@
+"""mask email for withdrawn; obtain details of messages sent to withdrawn users
+
+Revision ID: 5caf794c70a7
+Revises: 3c871e710277
+Create Date: 2024-03-25 12:14:46.377931
+
+"""
+from alembic import op
+import datetime
+import sqlalchemy as sa
+from flask import current_app
+from sqlalchemy.orm import sessionmaker
+
+from portal.models.audit import Audit
+from portal.models.message import EmailMessage
+from portal.models.organization import Organization, OrgTree
+from portal.models.qb_timeline import QBT
+from portal.models.user import User, WITHDRAWN_PREFIX, patients_query
+from portal.models.user_consent import UserConsent, consent_withdrawal_dates
+
+# revision identifiers, used by Alembic.
+revision = '5caf794c70a7'
+down_revision = '3c871e710277'
+
+Session = sessionmaker()
+
+
+def patients(admin):
+    """return list of patients potentially affected by withdrawal bugs
+
+    limited to IRONMAN, withdrawn, non-deleted patients
+    """
+    irnmn = Organization.query.filter(Organization.name == 'IRONMAN').first()
+    if not irnmn:
+        return
+
+    irnmn_orgs = OrgTree().here_and_below_id(organization_id=irnmn.id)
+
+    at_least_once_wd = UserConsent.query.filter(UserConsent.status == 'suspended').with_entities(
+        UserConsent.user_id).distinct()
+    return patients_query(
+        acting_user=admin,
+        include_test_role=False,
+        filter_by_ids=[i[0] for i in at_least_once_wd],
+        requested_orgs=irnmn_orgs).with_entities(User.id)
+
+
+def audit_since_for_patient(patient):
+    #version_pattern = '24.3.8'
+    min_id = 864688  # last id on prod with version 23.12.11.2
+    # ignore `assessment` audits, as that's primarily qb_timeline rebuilds
+    # which all of these users had.
+    audit_query = Audit.query.filter(Audit._context != 'assessment').filter(
+        Audit.subject_id == patient.id).filter(Audit.id > min_id)
+    if not audit_query.count():
+        return
+    for audit in audit_query:
+        if audit.context == 'consent' and audit.comment.startswith('remove bogus'):
+            continue
+        print(audit)
+
+def emails_since_for_patient(patient):
+    """report details on any emails since upgrade sent for patient"""
+    min_id = 140419  # last id on prod before update
+    addr = patient.email
+    email_by_addr = EmailMessage.query.filter(EmailMessage.recipients == addr).filter(
+        EmailMessage.id > min_id)
+    email_by_id = EmailMessage.query.filter(EmailMessage.recipient_id == patient.id).filter(
+        EmailMessage.id > min_id)
+    if not (email_by_addr.count() + email_by_id.count()):
+        return
+    ids_seen = set()
+
+    deceased = "deceased" if patient.deceased_id is not None else ""
+    print(f"Patient {patient.id} {patient.external_study_id} {deceased}")
+    for email in email_by_addr:
+        ids_seen.add(email.id)
+        print(f"User {patient.id} {email}")
+    for email in email_by_id:
+        if email.id not in ids_seen:
+            print(f"User {patient.id} {email}")
+
+
+def confirm_withdrawn_row(patient, rs_id):
+    """confirm a withdrawal row is in users qb_timeline for given research study"""
+    count = QBT.query.filter(QBT.user_id == patient.id).filter(
+        QBT.research_study_id == rs_id).filter(QBT.status == 'withdrawn').count()
+    if count != 1:
+        # look out for the one valid case, where user has zero timeline rows
+        if QBT.query.filter(QBT.user_id == patient.id).filter(
+                    QBT.research_study_id == rs_id).count() == 0:
+            return
+
+        raise ValueError(f"no (or too many) withdrawn row for {patient.id} {rs_id}")
+
+
+def mask_withdrawn_user_email(session, patient, admin):
+    if not patient.email_ready()[0]:
+        # no need, already hidden
+        return
+
+    version = current_app.config.metadata['version']
+    now = datetime.datetime.utcnow()
+    def audit_insert(subject_id):
+        msg = f"mask withdrawn user email"
+        insert = (
+            "INSERT INTO AUDIT"
+            " (user_id, subject_id, context, timestamp, version, comment)"
+            " VALUES"
+            f"({admin.id}, {subject_id}, 'user',"
+            f" '{now}', '{version}', '{msg}')")
+        session.execute(insert)
+
+    patient._email = WITHDRAWN_PREFIX + patient.email
+    audit_insert(patient.id)
+
+
+def upgrade():
+    bind = op.get_bind()
+    session = Session(bind=bind)
+    admin = session.query(User).filter_by(email='__system__').first()
+
+    for pat_row in patients(admin):
+        patient_id = pat_row[0]
+        patient = session.query(User).get(patient_id)
+        # confirm withdrawn status (global g or empro e)
+        consent_g, withdrawal_g = consent_withdrawal_dates(patient, 0)
+        consent_e, withdrawal_e = consent_withdrawal_dates(patient, 1)
+        if not (withdrawal_e or withdrawal_g):
+            continue
+
+        if withdrawal_g:
+            confirm_withdrawn_row(patient, 0)
+        if withdrawal_e:
+            confirm_withdrawn_row(patient, 1)
+
+        # check for users consented for both but only withdrawn from one
+        if (consent_g and consent_e) and not (withdrawal_g and withdrawal_e):
+            # print(f"user {patient_id} consented for both, but only withdrawn from one")
+            continue
+
+        audit_since_for_patient(patient)
+        emails_since_for_patient(patient)
+        mask_withdrawn_user_email(session, patient, admin)
+        session.commit()
+
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    pass
+    # ### end Alembic commands ###

portal/models/communication_request.py

@@ -249,7 +249,7 @@ def unfinished_work(qstats):
     def queue_communication(user, communication_request):
         """Create new communication object in preparation state"""
 
-        if not user.email or '@' not in user.email:
+        if not user.email_ready()[0]:
             raise ValueError(
                 "can't send communication to user w/o valid email address")
 

portal/models/qb_status.py

@@ -594,7 +594,7 @@ def patient_research_study_status(patient, ignore_QB_status=False):
                 # Clear ready status when base has pending work
                 rs_status['ready'] = False
                 rs_status['errors'].append('Pending work in base study')
-            elif not patient.email_ready():
+            elif not patient.email_ready()[0]:
                 # Avoid errors from automated emails, that is, email required
                 rs_status['ready'] = False
                 rs_status['errors'].append('User lacks valid email address')

portal/models/user.py

@@ -3,6 +3,7 @@
 import base64
 from html import escape
 from datetime import datetime, timedelta
+from dateutil.parser import parse as dateparser
 from io import StringIO
 import os
 import re
@@ -60,6 +61,7 @@
 
 INVITE_PREFIX = "__invite__"
 NO_EMAIL_PREFIX = "__no_email__"
+WITHDRAWN_PREFIX = "__withdrawn__"
 DELETED_PREFIX = "__deleted_{time}__"
 DELETED_REGEX = r"__deleted_\d+__(.*)"
 
@@ -641,6 +643,9 @@ def email_ready(self, ignore_preference=False):
         if self._email.startswith(NO_EMAIL_PREFIX) or not valid_email:
             return False, _("invalid email address")
 
+        if self._email.startswith(WITHDRAWN_PREFIX):
+            return False, _("withdrawn user; invalid address")
+
         if not ignore_preference and UserPreference.query.filter(
                 UserPreference.user_id == self.id).filter(
                 UserPreference.preference_name == 'suppress_email').first():
@@ -1279,6 +1284,7 @@ def update_consents(self, consent_list, acting_user):
             db.session.add(replaced)
         db.session.commit()
         self.check_consents()
+        self.mask_withdrawn()
 
     def check_consents(self):
         """Hook method for application of consent related rules"""
@@ -1310,6 +1316,68 @@ def check_consents(self):
                     "Multiple Primary Investigators for organization"
                     f" {consent.organization_id}")
 
+
+    def mask_withdrawn(self):
+        """withdrawn users get email mask to prevent any communication
+
+        after a few rounds of trouble with automated messages sneaking through,
+        now masking the email address to prevent any unintentional communications.
+
+        this method confirms the user was withdrawn from all studies, and if
+        so, adds an email maks.
+
+        alternatively, if the user has been reactivated and the withdrawn mask
+        is present, remove it.
+        """
+        from .research_study import EMPRO_RS_ID, BASE_RS_ID
+        from .user_consent import consent_withdrawal_dates
+
+        mask_user = None
+        # check former consent/withdrawal status for both studies
+        consent_g, withdrawal_g = consent_withdrawal_dates(self, BASE_RS_ID)
+        consent_e, withdrawal_e = consent_withdrawal_dates(self, EMPRO_RS_ID)
+
+        if not consent_g:
+            # never consented, done.
+            mask_user = False
+        if mask_user is None and not consent_e:
+            # only in global study
+            if withdrawal_g:
+                mask_user = True
+            else:
+                mask_user = False
+        elif mask_user is None:
+            # in both studies
+            if not withdrawal_g or not withdrawal_e:
+                # haven't withdrawn from both
+                mask_user = False
+            elif withdrawal_g and withdrawal_e:
+                # withdrawn from both
+                mask_user = True
+
+        # apply or remove mask if needed
+        comment = None
+        if mask_user is True:
+            if not self.email_ready()[0]:
+                # already masked or no email - no op
+                return
+            self._email = WITHDRAWN_PREFIX + self._email
+            comment = "mask withdrawn user email"
+        if mask_user is False:
+            if self._email and self._email.startswith(WITHDRAWN_PREFIX):
+                self._email = self._email[len(WITHDRAWN_PREFIX):]
+                comment = "remove withdrawn user email mask"
+
+        if comment:
+            audit = Audit(
+                user_id=self.id,
+                subject_id=self.id,
+                comment=comment,
+                context='user',
+                timestamp=datetime.utcnow())
+            db.session.add(audit)
+            db.session.commit()
+
     def deactivate_tous(self, acting_user, types=None):
         """ Mark user's current active ToU agreements as inactive
 
@@ -1502,8 +1570,7 @@ def update_roles(self, role_list, acting_user):
     def update_birthdate(self, fhir):
         try:
             bd = fhir['birthDate']
-            self.birthdate = datetime.strptime(
-                bd.strip(), '%Y-%m-%d') if bd else None
+            self.birthdate = dateparser(bd.strip()) if bd else None
         except (AttributeError, ValueError):
             abort(400, "birthDate '{}' doesn't match expected format "
                        "'%Y-%m-%d'".format(fhir['birthDate']))

portal/tasks.py

@@ -333,10 +333,14 @@ def send_user_messages(user, force_update=False):
     if force_update:
         for rs_id in users_rs_ids:
             invalidate_users_QBT(user_id=user.id, research_study_id=rs_id)
-            qbd = QB_Status(
+            qbstatus = QB_Status(
                 user=user,
                 research_study_id=rs_id,
-                as_of_date=datetime.utcnow()).current_qbd()
+                as_of_date=datetime.utcnow())
+            if qbstatus.withdrawn_by(datetime.utcnow()):
+                # NEVER notify withdrawn patients
+                continue
+            qbd = qbstatus.current_qbd()
             if qbd:
                 queue_outstanding_messages(
                     user=user,

portal/trigger_states/empro_messages.py

@@ -113,7 +113,7 @@ def staff_emails(patient, hard_triggers, initial_notification):
     }
     emails = []
     for staff in staff_list:
-        if not staff.email_ready():
+        if not staff.email_ready()[0]:
             current_app.logger.error(f"can't email staff {staff} without email")
             continue
         mr = MailResource(

portal/trigger_states/empro_states.py

@@ -292,7 +292,7 @@ def process_processed(ts):
         patient = User.query.get(ts.user_id)
 
         # Patient always gets mail
-        if patient.email_ready():
+        if patient.email_ready()[0]:
             pending_emails.append((
                 patient_email(patient, soft_triggers, hard_triggers),
                 "patient thank you"))

tests/__init__.py

@@ -63,6 +63,7 @@
 
 TEST_USER_ID = 1
 TEST_USERNAME = 'testy@example.com'
+TEST_BIRTHDAY = '1999-12-31'
 FIRST_NAME = '✓'
 LAST_NAME = 'Last'
 IMAGE_URL = 'http://examle.com/photo.jpg'
@@ -159,6 +160,7 @@ def init_data(self):
             test_user = self.add_user(
                 username=TEST_USERNAME, first_name=FIRST_NAME,
                 last_name=LAST_NAME, image_url=IMAGE_URL)
+            test_user.birthdate = TEST_BIRTHDAY
         except IntegrityError:
             db.session.rollback()
             test_user = User.query.filter_by(username=TEST_USERNAME).one()

tests/conftest.py

@@ -410,7 +410,8 @@ def staff_user(test_user, promote_user):
 
 @pytest.fixture
 def staff_rp_org_user(add_user, promote_user, org_w_test_rp):
-    staff = add_user(username="test_staff_user")
+    staff = add_user(username="test_staff_user", email="test_staff_user@example.com")
+    staff.birthdate = datetime(year=2020, month=1, day=1)
     promote_user(user=staff, role_name=ROLE.STAFF.value)
     promote_user(user=staff, role_name=ROLE.CLINICIAN.value)
     staff = db.session.merge(staff)

tests/test_consent.py

@@ -13,6 +13,7 @@
 from portal.models.audit import Audit
 from portal.models.organization import Organization
 from portal.models.research_study import ResearchStudy
+from portal.models.user import User, WITHDRAWN_PREFIX
 from portal.models.user_consent import UserConsent
 from tests import TEST_USER_ID, TestCase
 
@@ -377,6 +378,10 @@ def test_delete_user_consent(self):
             user_id=TEST_USER_ID, organization_id=org1_id).first()
         assert dc.status == 'deleted'
 
+        # confirm withdrawn user email mask not in place
+        self.test_user = db.session.query(User).get(TEST_USER_ID)
+        assert not self.test_user._email.startswith(WITHDRAWN_PREFIX)
+
     def test_withdraw_user_consent(self):
         self.shallow_org_tree()
         org = Organization.query.filter(Organization.id > 0).first()
@@ -421,6 +426,10 @@ def test_withdraw_user_consent(self):
         assert not new_consent.send_reminders
         assert new_consent.acceptance_date == suspend_date
 
+        # confirm withdrawn user email mask in place
+        self.test_user = db.session.query(User).get(TEST_USER_ID)
+        assert self.test_user._email.startswith(WITHDRAWN_PREFIX)
+
     def test_withdraw_user_consent_other_study(self):
         self.shallow_org_tree()
         org = Organization.query.filter(Organization.id > 0).first()
@@ -491,6 +500,10 @@ def test_withdraw_user_consent_other_study(self):
         assert valid_consents[1].research_study_id == 1
         assert valid_consents[1].acceptance_date == study_1_acceptance_date
 
+        # confirm user email isn't masked
+        self.test_user = db.session.query(User).get(TEST_USER_ID)
+        assert not self.test_user._email.startswith(WITHDRAWN_PREFIX)
+
     def test_withdraw_too_early(self):
         """Avoid problems with withdrawals predating the existing consent"""
         self.shallow_org_tree()
@@ -587,3 +600,7 @@ def test_double_withdrawal(self):
             status='suspended')
         assert query.count() == 1
         assert query.first().acceptance_date == just_right
+
+        # confirm withdrawn user email mask in place
+        self.test_user = db.session.query(User).get(TEST_USER_ID)
+        assert self.test_user._email.startswith(WITHDRAWN_PREFIX)