From 919ce3a80765ebb62748f4330599a8fd3e33878e Mon Sep 17 00:00:00 2001 From: Amy Chen Date: Tue, 15 Oct 2024 17:10:27 -0700 Subject: [PATCH 1/3] Fixup/table preference (#4411) Address a stream of 400 errors (CSRF token is missing) related to saving patientlist table preferences. Cause: I think the error stemmed from when an ajax request was first made to retrieve patient data, which is precedes by a call to save table preferences, the document DOM was not ready (which includes the value of CSRF token in an hidden element) - hence the missing CSRF token error. Fix: To add check for DOM readiness before the request to save table preferences is made. --------- Co-authored-by: Amy Chen --- portal/static/js/src/admin.js | 3 ++- portal/static/js/src/modules/TnthAjax.js | 10 ++++++++++ portal/templates/admin/admin_base.html | 2 ++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/portal/static/js/src/admin.js b/portal/static/js/src/admin.js index 8d757c186..391a5f218 100644 --- a/portal/static/js/src/admin.js +++ b/portal/static/js/src/admin.js @@ -1323,7 +1323,8 @@ let requestTimerId = 0; ) { var tnthAjax = this.getDependency("tnthAjax"); tableName = tableName || this.tableIdentifier; - if (!tableName) { + if (!tableName || !document.querySelector("#adminTable")) { + if (callback) callback(); 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 f6cbad11d..72b46ff42 100644 --- a/portal/static/js/src/modules/TnthAjax.js +++ b/portal/static/js/src/modules/TnthAjax.js @@ -93,6 +93,16 @@ 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 4d4db9f8a..94d851c85 100644 --- a/portal/templates/admin/admin_base.html +++ b/portal/templates/admin/admin_base.html @@ -93,6 +93,8 @@ // 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); From c89838365857c7ac82d850e7847f8ec7b7d680b2 Mon Sep 17 00:00:00 2001 From: Paul Bugni Date: Tue, 22 Oct 2024 11:43:47 -0700 Subject: [PATCH 2/3] TN-3324 date fields in patient list don't work to filter on partial strings. --- portal/views/patients.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/portal/views/patients.py b/portal/views/patients.py index f4fe7705d..3d1d97b4e 100644 --- a/portal/views/patients.py +++ b/portal/views/patients.py @@ -109,6 +109,10 @@ 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 From f1cb1c83d1ad57b5ebc7492d94c41509214e8daa Mon Sep 17 00:00:00 2001 From: pbugni Date: Wed, 23 Oct 2024 16:18:01 -0700 Subject: [PATCH 3/3] 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 | 88 ++++++++-------- portal/models/qb_status.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 +- 7 files changed, 52 insertions(+), 263 deletions(-) delete mode 100644 tests/integration_tests/__init__.py delete mode 100644 tests/integration_tests/pages.py delete mode 100644 tests/integration_tests/test_login.py diff --git a/portal/models/patient_list.py b/portal/models/patient_list.py index 9d693bf97..61e7a9e1b 100644 --- a/portal/models/patient_list.py +++ b/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() diff --git a/portal/models/qb_status.py b/portal/models/qb_status.py index f69dfb3ba..974dc4961 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).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( diff --git a/requirements.dev.txt b/requirements.dev.txt index 63c3eb52b..b4b826176 100644 --- a/requirements.dev.txt +++ b/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 . diff --git a/tests/integration_tests/__init__.py b/tests/integration_tests/__init__.py deleted file mode 100644 index e4f5f3b30..000000000 --- a/tests/integration_tests/__init__.py +++ /dev/null @@ -1,141 +0,0 @@ -"""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 deleted file mode 100644 index 652e5315c..000000000 --- a/tests/integration_tests/pages.py +++ /dev/null @@ -1,8 +0,0 @@ -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 deleted file mode 100644 index 249a78290..000000000 --- a/tests/integration_tests/test_login.py +++ /dev/null @@ -1,64 +0,0 @@ -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 49dbdd727..ed0a6ecc5 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 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'