diff --git a/portal/migrations/versions/5caf794c70a7_.py b/portal/migrations/versions/5caf794c70a7_.py new file mode 100644 index 000000000..5e911076d --- /dev/null +++ b/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 ### diff --git a/portal/models/communication_request.py b/portal/models/communication_request.py index 37a41709b..a54067d8a 100644 --- a/portal/models/communication_request.py +++ b/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") diff --git a/portal/models/qb_status.py b/portal/models/qb_status.py index c6cf1e774..6e89670e9 100644 --- a/portal/models/qb_status.py +++ b/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') diff --git a/portal/models/user.py b/portal/models/user.py index b39de7116..517c33ebc 100644 --- a/portal/models/user.py +++ b/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'])) diff --git a/portal/tasks.py b/portal/tasks.py index a24021c40..6cfb1f19e 100644 --- a/portal/tasks.py +++ b/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, diff --git a/portal/trigger_states/empro_messages.py b/portal/trigger_states/empro_messages.py index 0ad1c5015..d0f2fb99d 100644 --- a/portal/trigger_states/empro_messages.py +++ b/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( diff --git a/portal/trigger_states/empro_states.py b/portal/trigger_states/empro_states.py index a2c73e52f..7ce15e37c 100644 --- a/portal/trigger_states/empro_states.py +++ b/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")) diff --git a/tests/__init__.py b/tests/__init__.py index a13fd9e06..8a7beb2de 100644 --- a/tests/__init__.py +++ b/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() diff --git a/tests/conftest.py b/tests/conftest.py index 1312cf4d8..7f6d58e8a 100644 --- a/tests/conftest.py +++ b/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) diff --git a/tests/test_consent.py b/tests/test_consent.py index 77000eba8..59b92daf1 100644 --- a/tests/test_consent.py +++ b/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) diff --git a/tests/test_demographics.py b/tests/test_demographics.py index 9f035efce..eb1e5e6be 100644 --- a/tests/test_demographics.py +++ b/tests/test_demographics.py @@ -271,14 +271,14 @@ def test_demographics_duplicate_email_different_case(self): user = User.query.get(TEST_USER_ID) assert user._email == NO_EMAIL_PREFIX - def test_demographics_bad_dob(self): + def test_demographics_alternative_format_dob(self): data = {"resourceType": "Patient", "birthDate": '10/20/1980'} self.login() response = self.client.put( '/api/demographics/%s' % TEST_USER_ID, content_type='application/json', data=json.dumps(data)) - assert response.status_code == 400 + assert response.status_code == 200 def test_demographics_list_names(self): # confirm we can handle when given lists for names as spec'd