From b8a4bd613c5fb3cb27e1416ae9a932de656220b7 Mon Sep 17 00:00:00 2001 From: pbugni Date: Wed, 23 Oct 2024 19:46:23 -0700 Subject: [PATCH] Revert "TN-3324 robust handling of unsortable patient list attributes (#4414)" This reverts commit 212e2444cd511fd98006035c8724342a280c9912. --- portal/models/patient_list.py | 88 +++++++------- portal/models/qb_status.py | 4 +- portal/static/js/src/admin.js | 3 +- portal/static/js/src/modules/TnthAjax.js | 10 -- portal/templates/admin/admin_base.html | 2 - portal/views/patients.py | 4 - requirements.dev.txt | 2 + tests/integration_tests/__init__.py | 141 +++++++++++++++++++++++ tests/integration_tests/pages.py | 8 ++ tests/integration_tests/test_login.py | 64 ++++++++++ tests/test_scheduled_job.py | 8 +- 11 files changed, 264 insertions(+), 70 deletions(-) create mode 100644 tests/integration_tests/__init__.py create mode 100644 tests/integration_tests/pages.py create mode 100644 tests/integration_tests/test_login.py diff --git a/portal/models/patient_list.py b/portal/models/patient_list.py index 61e7a9e1b9..9d693bf972 100644 --- a/portal/models/patient_list.py +++ b/portal/models/patient_list.py @@ -53,53 +53,49 @@ def patient_list_update_patient(patient_id, research_study_id=None): if not user.has_role(ROLE.PATIENT.value): return - 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) + 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() diff --git a/portal/models/qb_status.py b/portal/models/qb_status.py index 974dc49610..f69dfb3ba1 100644 --- a/portal/models/qb_status.py +++ b/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).with_entities(QBT.id).first() - self.at_least_one_completed = completed is not None + QBT.status == OverallStatus.completed).count() + self.at_least_one_completed = completed > 0 # Obtain withdrawal date if applicable withdrawn = QBT.query.filter(QBT.user_id == self.user.id).filter( diff --git a/portal/static/js/src/admin.js b/portal/static/js/src/admin.js index 391a5f218d..8d757c186a 100644 --- a/portal/static/js/src/admin.js +++ b/portal/static/js/src/admin.js @@ -1323,8 +1323,7 @@ let requestTimerId = 0; ) { var tnthAjax = this.getDependency("tnthAjax"); tableName = tableName || this.tableIdentifier; - if (!tableName || !document.querySelector("#adminTable")) { - if (callback) callback(); + if (!tableName) { return false; } userId = userId || this.userId; diff --git a/portal/static/js/src/modules/TnthAjax.js b/portal/static/js/src/modules/TnthAjax.js index 72b46ff42e..f6cbad11d9 100644 --- a/portal/static/js/src/modules/TnthAjax.js +++ b/portal/static/js/src/modules/TnthAjax.js @@ -93,16 +93,6 @@ export default { /*global $ */ $.ajax("/api/me").done( function() { console.log("user authorized"); - if ((typeof CsrfTokenChecker !== "undefined") && - !CsrfTokenChecker.checkTokenValidity()) { - //if CSRF Token not valid, return error - if (callback) { - callback({"error": DEFAULT_SERVER_DATA_ERROR}); - fieldHelper.showError(targetField); - } - return; - } - ajaxCall(); } ).fail(function() { diff --git a/portal/templates/admin/admin_base.html b/portal/templates/admin/admin_base.html index 94d851c851..4d4db9f8a0 100644 --- a/portal/templates/admin/admin_base.html +++ b/portal/templates/admin/admin_base.html @@ -93,8 +93,6 @@ // custom ajax request here function patientDataAjaxRequest(params) { loadIntervalId = setInterval(() => { - //document DOM not ready, don't make ajax call yet - if (!document.querySelector("#adminTable")) return; if (typeof window.AdminObj === "undefined") return; window.AdminObj.getRemotePatientListData(params); clearInterval(loadIntervalId); diff --git a/portal/views/patients.py b/portal/views/patients.py index 3d1d97b4e6..f4fe7705dc 100644 --- a/portal/views/patients.py +++ b/portal/views/patients.py @@ -109,10 +109,6 @@ def filter_query(query, filter_field, filter_value): # ignore requests to filter by unknown column return query - if filter_field in ('birthdate', 'consentdate', 'empro_consentdate'): - # these are not filterable (partial strings on date complexity) - ignore such a request - return query - if filter_field == 'userid': query = query.filter(PatientList.userid == int(filter_value)) return query diff --git a/requirements.dev.txt b/requirements.dev.txt index b4b826176b..63c3eb52bb 100644 --- a/requirements.dev.txt +++ b/requirements.dev.txt @@ -20,6 +20,7 @@ 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 @@ -35,4 +36,5 @@ 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 . diff --git a/tests/integration_tests/__init__.py b/tests/integration_tests/__init__.py new file mode 100644 index 0000000000..e4f5f3b301 --- /dev/null +++ b/tests/integration_tests/__init__.py @@ -0,0 +1,141 @@ +"""Unit test module for Selenium testing""" + +import os +import sys +import unittest + +from flask_testing import LiveServerTestCase +import pytest +from selenium import webdriver +from selenium.webdriver.support.ui import WebDriverWait +import xvfbwrapper + +from tests import TestCase + + +@unittest.skipUnless( + ( + "SAUCE_USERNAME" in os.environ or + xvfbwrapper.Xvfb().xvfb_exists() + ), + "Xvfb not installed" +) +class IntegrationTestCase(TestCase, LiveServerTestCase): + """Test class for UI integration/workflow testing""" + + def setUp(self): + """Reset all tables before testing.""" + + if "SAUCE_USERNAME" in os.environ: + # Configure driver for Sauce Labs + # Presumes tunnel setup by Sauce Connect + # On TravisCI, Sauce Connect tunnel setup by Sauce Labs addon + # https://docs.travis-ci.com/user/sauce-connect + platform = { + "browserName": "firefox", + "platform": "Windows 10", + "version": "60.0", + } + capabilities = { + "tunnel-identifier": os.environ["TRAVIS_JOB_NUMBER"], + "extendedDebugging": "true", + } + metadata = { + "name": self.id(), + "build": "#%s %s" % ( + os.environ["TRAVIS_BUILD_NUMBER"], + os.environ["TRAVIS_BRANCH"], + ), + "tags": [ + "py" + os.environ["TRAVIS_PYTHON_VERSION"], + "CI", + ], + "passed": False, + } + capabilities.update(platform) + capabilities.update(metadata) + + url = "http://{user}:{access_key}@localhost:4445/wd/hub".format( + user=os.environ["SAUCE_USERNAME"], + access_key=os.environ["SAUCE_ACCESS_KEY"], + ) + + self.driver = webdriver.Remote( + desired_capabilities=capabilities, + command_executor=url + ) + + else: + if "DISPLAY" not in os.environ: + # Non-graphical environment; use xvfb + self.xvfb = xvfbwrapper.Xvfb() + self.addCleanup(self.xvfb.stop) + self.xvfb.start() + self.driver = webdriver.Firefox(timeout=60) + + self.addCleanup(self.driver.quit) + + self.driver.root_uri = self.get_server_url() + self.driver.implicitly_wait(30) + self.verificationErrors = [] + # default explicit wait time; use with Expected Conditions as needed + self.wait = WebDriverWait(self.driver, 60) + self.accept_next_alert = True + + super(IntegrationTestCase, self).setUp() + + def is_element_present(self, how, what): + """Detects whether or not an element can be found in DOM + + This function was exported from Selenium IDE + """ + try: + self.driver.find_element(by=how, value=what) + except NoSuchElementException as e: + return False + return True + + def is_alert_present(self): + """Detects whether an alert message is present + + This function was exported from Selenium IDE + """ + try: + self.driver.switch_to_alert() + except NoAlertPresentException as e: + return False + return True + + def close_alert_and_get_its_text(self): + """Closes an alert, if present, and returns its text + + If an alert is not present a NoAlertPresentException + will be thrown. + This function was exported from Selenium IDE + """ + try: + alert = self.driver.switch_to_alert() + alert_text = alert.text + if self.accept_next_alert: + alert.accept() + else: + alert.dismiss() + return alert_text + finally: + self.accept_next_alert = True + + def tearDown(self): + """Clean db session, drop all tables.""" + + # Update job result metadata on Sauce Labs, if available + if ( + "SAUCE_USERNAME" in os.environ and + + # No exception being handled - test completed successfully + sys.exc_info() == (None, None, None) + ): + self.driver.execute_script("sauce:job-result=passed") + + self.assertEqual([], self.verificationErrors) + + super(IntegrationTestCase, self).tearDown() diff --git a/tests/integration_tests/pages.py b/tests/integration_tests/pages.py new file mode 100644 index 0000000000..652e5315c3 --- /dev/null +++ b/tests/integration_tests/pages.py @@ -0,0 +1,8 @@ +from page_objects import PageElement, PageObject + + +class LoginPage(PageObject): + username = PageElement(name='email') + password = PageElement(name='password') + login_button = PageElement(css='input[type="submit"]') + facebook_button = PageElement(css='.btn-facebook') diff --git a/tests/integration_tests/test_login.py b/tests/integration_tests/test_login.py new file mode 100644 index 0000000000..249a78290d --- /dev/null +++ b/tests/integration_tests/test_login.py @@ -0,0 +1,64 @@ +from flask import url_for +import pytest +from selenium.webdriver.support.ui import Select + +from tests import DEFAULT_PASSWORD, TEST_USERNAME +from tests.integration_tests import IntegrationTestCase + + +class TestLogin(IntegrationTestCase): + """Test class for Login integration tests""" + + def test_login_page(self): + """Ensure login works properly""" + driver = self.driver + driver.get(url_for("user.login", _external=True)) + + driver.find_element_by_name("email").click() + driver.find_element_by_name("email").clear() + driver.find_element_by_name("email").send_keys(TEST_USERNAME) + driver.find_element_by_name("password").click() + driver.find_element_by_name("password").clear() + driver.find_element_by_name("password").send_keys(DEFAULT_PASSWORD) + driver.find_element_by_xpath( + "//input[@class='btn btn-tnth-primary btn-lg' and @value='LOG IN']" + ).click() + driver.find_element_by_id("tnthUserBtn").click() + driver.find_element_by_link_text("Log Out of TrueNTH").click() + + @pytest.mark.skip(reason="not able to complete consent page") + def test_consent_after_login(self): + driver = self.driver + driver.get(url_for("eproms.home", _external=True)) + driver.find_element_by_id("email").click() + driver.find_element_by_id("email").clear() + driver.find_element_by_id("email").send_keys(TEST_USERNAME) + driver.find_element_by_id("password").click() + driver.find_element_by_id("password").clear() + driver.find_element_by_id("password").send_keys(DEFAULT_PASSWORD) + driver.find_element_by_id("btnLogin").click() + driver.find_element_by_xpath("(.//*[normalize-space(text()) and normalize-space(.)='General Terms of Use'])[1]/following::i[1]").click() + driver.find_element_by_xpath("(.//*[normalize-space(text()) and normalize-space(.)='General Terms of Use'])[1]/following::i[2]").click() + driver.find_element_by_id("next").click() + driver.find_element_by_id("firstname").clear() + driver.find_element_by_id("firstname").send_keys("Test") + driver.find_element_by_id("lastname").click() + driver.find_element_by_id("lastname").clear() + driver.find_element_by_id("lastname").send_keys("User") + driver.find_element_by_id("date").click() + driver.find_element_by_id("month").click() + driver.find_element_by_xpath("(.//*[normalize-space(text()) and normalize-space(.)='(optional)'])[1]/following::option[11]").click() + driver.find_element_by_id("year").click() + driver.find_element_by_id("year").clear() + driver.find_element_by_id("year").send_keys("1988") + driver.find_element_by_id("role_patient").click() + driver.find_element_by_id("next").click() + driver.find_element_by_id("biopsy_no").click() + driver.find_element_by_id("next").click() + driver.find_element_by_id("stateSelector").click() + Select(driver.find_element_by_id("stateSelector")).select_by_visible_text("Washington") + driver.find_element_by_xpath("(.//*[normalize-space(text()) and normalize-space(.)='Your clinic of care.'])[1]/following::option[12]").click() + driver.find_element_by_xpath("(.//*[normalize-space(text()) and normalize-space(.)='UW Medicine (University of Washington)'])[1]/following::label[1]").click() + driver.find_element_by_id("updateProfile").click() + driver.find_element_by_id("tnthUserBtn").click() + driver.find_element_by_link_text("Log Out of TrueNTH").click() diff --git a/tests/test_scheduled_job.py b/tests/test_scheduled_job.py index ed0a6ecc54..49dbdd7279 100644 --- a/tests/test_scheduled_job.py +++ b/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 task_test +from portal.tasks import test as test_task 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 = task_test(**kdict) + resp = test_task(**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 = task_test(**kdict) + resp = test_task(**kdict) assert len(resp.split()) == 4 assert resp.split()[-1] == 'inactive.' # test manual override run of inactive job kdict['manual_run'] = True - resp = task_test(**kdict) + resp = test_task(**kdict) assert len(resp.split()) == 6 assert resp.split()[-1] == 'Test'