Fixes found in failing testing details (#4416)

Addressed a number of warnings in the tests, and a few random
non-repeatable failures. Tests look to be more stable again.

This does include the elimination of "integration_tests" which used
selenium to attempt some basic browser login and such verification
tests. They are fragile, and of little use given our required test plan
before any release.

portal/models/patient_list.py

@@ -53,49 +53,53 @@ def patient_list_update_patient(patient_id, research_study_id=None):
     if not user.has_role(ROLE.PATIENT.value):
         return
 
-    patient = PatientList.query.get(patient_id)
-    new_record = False
-    if not patient:
-        new_record = True
-        patient = PatientList(userid=patient_id)
-        db.session.add(patient)
+    from ..timeout_lock import TimeoutLock
+    # async possibility, only allow one thread at a time
+    key = f"patient_list_update_patient:{patient_id}"
+    with TimeoutLock(key, expires=60, timeout=60):
+        patient = PatientList.query.get(patient_id)
+        new_record = False
+        if not patient:
+            new_record = True
+            patient = PatientList(userid=patient_id)
+            db.session.add(patient)
 
-    if research_study_id is None or new_record:
-        patient.study_id = user.external_study_id
-        patient.firstname = user.first_name
-        patient.lastname = user.last_name
-        patient.email = user.email
-        patient.birthdate = user.birthdate
-        patient.deleted = user.deleted_id is not None
-        patient.test_role = True if user.has_role(ROLE.TEST.value) else False
-        patient.org_id = user.organizations[0].id if user.organizations else None
-        patient.org_name = user.organizations[0].name if user.organizations else None
+        if research_study_id is None or new_record:
+            patient.study_id = user.external_study_id
+            patient.firstname = user.first_name
+            patient.lastname = user.last_name
+            patient.email = user.email
+            patient.birthdate = user.birthdate
+            patient.deleted = user.deleted_id is not None
+            patient.test_role = True if user.has_role(ROLE.TEST.value) else False
+            patient.org_id = user.organizations[0].id if user.organizations else None
+            patient.org_name = user.organizations[0].name if user.organizations else None
 
-    # necessary to avoid recursive loop via some update paths
-    now = datetime.utcnow()
-    if patient.last_updated and patient.last_updated + timedelta(seconds=10) > now:
-        db.session.commit()
-        return
+        # necessary to avoid recursive loop via some update paths
+        now = datetime.utcnow()
+        if patient.last_updated and patient.last_updated + timedelta(seconds=10) > now:
+            db.session.commit()
+            return
 
-    patient.last_updated = now
-    if research_study_id == BASE_RS_ID or research_study_id is None:
-        rs_id = BASE_RS_ID
-        qb_status = qb_status_visit_name(
-             patient.userid, research_study_id=rs_id, as_of_date=now)
-        patient.questionnaire_status = str(qb_status['status'])
-        patient.visit = qb_status['visit_name']
-        patient.consentdate, _ = consent_withdrawal_dates(user=user, research_study_id=rs_id)
+        patient.last_updated = now
+        if research_study_id == BASE_RS_ID or research_study_id is None:
+            rs_id = BASE_RS_ID
+            qb_status = qb_status_visit_name(
+                 patient.userid, research_study_id=rs_id, as_of_date=now)
+            patient.questionnaire_status = str(qb_status['status'])
+            patient.visit = qb_status['visit_name']
+            patient.consentdate, _ = consent_withdrawal_dates(user=user, research_study_id=rs_id)
 
-    if (research_study_id == EMPRO_RS_ID or research_study_id is None) and user.clinicians:
-        rs_id = EMPRO_RS_ID
-        patient.clinician = '; '.join(
-            (clinician_name_map().get(c.id, "not in map") for c in user.clinicians)) or ""
-        qb_status = qb_status_visit_name(
-            patient.userid, research_study_id=rs_id, as_of_date=now)
-        patient.empro_status = str(qb_status['status'])
-        patient.empro_visit = qb_status['visit_name']
-        patient.action_state = qb_status['action_state'].title() \
-            if qb_status['action_state'] else ""
-        patient.empro_consentdate, _ = consent_withdrawal_dates(
-            user=user, research_study_id=rs_id)
-    db.session.commit()
+        if (research_study_id == EMPRO_RS_ID or research_study_id is None) and user.clinicians:
+            rs_id = EMPRO_RS_ID
+            patient.clinician = '; '.join(
+                (clinician_name_map().get(c.id, "not in map") for c in user.clinicians)) or ""
+            qb_status = qb_status_visit_name(
+                patient.userid, research_study_id=rs_id, as_of_date=now)
+            patient.empro_status = str(qb_status['status'])
+            patient.empro_visit = qb_status['visit_name']
+            patient.action_state = qb_status['action_state'].title() \
+                if qb_status['action_state'] else ""
+            patient.empro_consentdate, _ = consent_withdrawal_dates(
+                user=user, research_study_id=rs_id)
+        db.session.commit()

portal/models/qb_status.py

@@ -49,8 +49,8 @@ def _sync_timeline(self):
 
         completed = QBT.query.filter(QBT.user_id == self.user.id).filter(
             QBT.research_study_id == self.research_study_id).filter(
-            QBT.status == OverallStatus.completed).count()
-        self.at_least_one_completed = completed > 0
+            QBT.status == OverallStatus.completed).with_entities(QBT.id).first()
+        self.at_least_one_completed = completed is not None
 
         # Obtain withdrawal date if applicable
         withdrawn = QBT.query.filter(QBT.user_id == self.user.id).filter(

requirements.dev.txt

@@ -20,7 +20,6 @@ pyparsing==2.4.7          # via packaging
 pytest >= 6.1.0
 pytest-flask==0.15.1
 pytest-timeout==1.4.2
-selenium==3.141.0
 snowballstemmer==2.0.0    # via sphinx
 sphinx-rtd-theme==0.5.1
 sphinx==3.3.1
@@ -36,5 +35,4 @@ virtualenv==20.4.2        # via tox
 waitress==1.4.3           # via webtest
 webob==1.8.5              # via webtest
 webtest==2.0.33           # via flask-webtest
-xvfbwrapper==0.2.9
 --editable .

tests/test_scheduled_job.py

@@ -8,7 +8,7 @@
 from portal.extensions import db
 from portal.models.role import ROLE
 from portal.models.scheduled_job import ScheduledJob
-from portal.tasks import test as test_task
+from portal.tasks import test as task_test
 from sqlalchemy.orm import session
 from tests import TestCase
 
@@ -122,7 +122,7 @@ def test_active_check(self):
         job = db.session.merge(job)
 
         kdict = {"job_id": job.id}
-        resp = test_task(**kdict)
+        resp = task_test(**kdict)
         assert len(resp.split()) == 6
         assert resp.split()[-1] == 'Test'
 
@@ -135,13 +135,13 @@ def test_active_check(self):
         job = db.session.merge(job)
 
         kdict = {"job_id": job.id}
-        resp = test_task(**kdict)
+        resp = task_test(**kdict)
         assert len(resp.split()) == 4
         assert resp.split()[-1] == 'inactive.'
 
         # test manual override run of inactive job
         kdict['manual_run'] = True
-        resp = test_task(**kdict)
+        resp = task_test(**kdict)
         assert len(resp.split()) == 6
         assert resp.split()[-1] == 'Test'