From b498a1fc91b7b57a8fec15799e4b4defbb0624a6 Mon Sep 17 00:00:00 2001 From: OptimusRhine Date: Fri, 1 Dec 2017 16:23:32 -0800 Subject: [PATCH 01/19] lookup QNR based on QB for assessment_status --- portal/models/assessment_status.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/portal/models/assessment_status.py b/portal/models/assessment_status.py index cb86669116..bcde6d9da3 100644 --- a/portal/models/assessment_status.py +++ b/portal/models/assessment_status.py @@ -11,11 +11,12 @@ from .user import User -def recent_qnr_status(user, questionnaire_name): +def recent_qnr_status(user, questionnaire_name, qb): """Look up recent status/timestamp for matching QuestionnaireResponse :param user: Patient to whom completed QuestionnaireResponses belong :param questionnaire_name: name of associated questionnaire + :param qb: QuestionnaireBank of associated questionnaire :return: dictionary with authored (timestamp) of the most recent QuestionnaireResponse keyed by status found @@ -26,7 +27,8 @@ def recent_qnr_status(user, questionnaire_name): ).filter( QuestionnaireResponse.document[ ('questionnaire', 'reference') - ].astext.endswith(questionnaire_name) + ].astext.endswith(questionnaire_name), + QuestionnaireResponse.questionnaire_bank_id == qb.id ).order_by( QuestionnaireResponse.status, QuestionnaireResponse.authored).limit(9).with_entities( @@ -96,7 +98,7 @@ def qb_status_dict(user, questionnaire_bank, as_of_date): expired = questionnaire_bank.calculated_expiry( trigger_date, as_of_date=as_of_date) for q in questionnaire_bank.questionnaires: - recents = recent_qnr_status(user, q.name) + recents = recent_qnr_status(user, q.name, questionnaire_bank) d[q.name] = status_from_recents( recents, start, overdue, expired, as_of_date=as_of_date) trace("QuestionnaireBank status for {}:".format(questionnaire_bank.name)) From c0b6963bfe24f4507c2c14ec8ba8e032c94bcc2a Mon Sep 17 00:00:00 2001 From: OptimusRhine Date: Fri, 1 Dec 2017 16:24:03 -0800 Subject: [PATCH 02/19] update existing tests (incl. simplifying mock_qr() method --- tests/test_assessment_status.py | 65 ++++++++++++++++----------------- tests/test_communication.py | 30 ++++++--------- tests/test_intervention.py | 4 +- 3 files changed, 46 insertions(+), 53 deletions(-) diff --git a/tests/test_assessment_status.py b/tests/test_assessment_status.py index 19819ba183..bd4d27224d 100644 --- a/tests/test_assessment_status.py +++ b/tests/test_assessment_status.py @@ -17,10 +17,11 @@ from portal.models.recur import Recur from portal.models.research_protocol import ResearchProtocol from portal.models.role import ROLE +from portal.models.user import get_user from tests import TestCase, TEST_USER_ID -def mock_qr(user_id, instrument_id, status='completed', timestamp=None): +def mock_qr(instrument_id, status='completed', timestamp=None, qb=None): timestamp = timestamp or datetime.utcnow() qr_document = { "questionnaire": { @@ -36,16 +37,19 @@ def mock_qr(user_id, instrument_id, status='completed', timestamp=None): db.session.add(enc) db.session.commit() enc = db.session.merge(enc) + qb = qb or QuestionnaireBank.most_current_qb(get_user(TEST_USER_ID), + timestamp).questionnaire_bank qr = QuestionnaireResponse( subject_id=TEST_USER_ID, status=status, authored=timestamp, document=qr_document, - encounter_id=enc.id) + encounter_id=enc.id, + questionnaire_bank=qb) with SessionScope(db): db.session.add(qr) db.session.commit() - invalidate_assessment_status_cache(user_id) + invalidate_assessment_status_cache(TEST_USER_ID) localized_instruments = set(['eproms_add', 'epic26', 'comorb']) @@ -332,9 +336,9 @@ def test_localized_on_time(self): # User finished both on time self.bless_with_basics() # pick up a consent, etc. self.mark_localized() - mock_qr(user_id=TEST_USER_ID, instrument_id='eproms_add') - mock_qr(user_id=TEST_USER_ID, instrument_id='epic26') - mock_qr(user_id=TEST_USER_ID, instrument_id='comorb') + mock_qr(instrument_id='eproms_add') + mock_qr(instrument_id='epic26') + mock_qr(instrument_id='comorb') self.test_user = db.session.merge(self.test_user) a_s = AssessmentStatus(user=self.test_user, as_of_date=None) @@ -348,12 +352,9 @@ def test_localized_inprogress_on_time(self): # User finished both on time self.bless_with_basics() # pick up a consent, etc. self.mark_localized() - mock_qr(user_id=TEST_USER_ID, instrument_id='eproms_add', - status='in-progress') - mock_qr(user_id=TEST_USER_ID, instrument_id='epic26', - status='in-progress') - mock_qr(user_id=TEST_USER_ID, instrument_id='comorb', - status='in-progress') + mock_qr(instrument_id='eproms_add', status='in-progress') + mock_qr(instrument_id='epic26', status='in-progress') + mock_qr(instrument_id='comorb', status='in-progress') self.test_user = db.session.merge(self.test_user) a_s = AssessmentStatus(user=self.test_user, as_of_date=None) @@ -368,7 +369,7 @@ def test_localized_in_process(self): # User finished one, time remains for other self.bless_with_basics() # pick up a consent, etc. self.mark_localized() - mock_qr(user_id=TEST_USER_ID, instrument_id='eproms_add') + mock_qr(instrument_id='eproms_add') self.test_user = db.session.merge(self.test_user) a_s = AssessmentStatus(user=self.test_user, as_of_date=None) @@ -384,11 +385,13 @@ def test_localized_in_process(self): def test_metastatic_on_time(self): # User finished both on time self.bless_with_basics() # pick up a consent, etc. + self.mark_metastatic() for i in metastatic_baseline_instruments: - mock_qr(user_id=TEST_USER_ID, instrument_id=i) - mock_qr(user_id=TEST_USER_ID, instrument_id='irondemog') + mock_qr(instrument_id=i) + mi_qb = QuestionnaireBank.query.filter_by( + name='metastatic_indefinite').first() + mock_qr(instrument_id='irondemog', qb=mi_qb) - self.mark_metastatic() self.test_user = db.session.merge(self.test_user) a_s = AssessmentStatus(user=self.test_user, as_of_date=None) self.assertEquals(a_s.overall_status, "Completed") @@ -421,11 +424,11 @@ def test_localized_overdue(self): # if the user completed something on time, and nothing else # is due, should see the thankyou message. - # backdate so the baseline q's have expired - mock_qr(user_id=TEST_USER_ID, instrument_id='epic26', - status='in-progress') self.bless_with_basics(backdate=relativedelta(months=3)) self.mark_localized() + # backdate so the baseline q's have expired + mock_qr(instrument_id='epic26', status='in-progress') + self.test_user = db.session.merge(self.test_user) a_s = AssessmentStatus(user=self.test_user, as_of_date=None) self.assertEquals(a_s.overall_status, "Partially Completed") @@ -440,12 +443,12 @@ def test_localized_as_of_date(self): # backdating consent beyond expired and the status lookup date # within a valid window should show available assessments. - # backdate so the baseline q's have expired - mock_qr(user_id=TEST_USER_ID, instrument_id='epic26', - status='in-progress', - timestamp=datetime.utcnow() - relativedelta(months=3)) self.bless_with_basics(backdate=relativedelta(months=3)) self.mark_localized() + # backdate so the baseline q's have expired + mock_qr(instrument_id='epic26', status='in-progress', + timestamp=datetime.utcnow() - relativedelta(months=3)) + self.test_user = db.session.merge(self.test_user) as_of_date = datetime.utcnow() - relativedelta(months=2, days=28) a_s = AssessmentStatus(user=self.test_user, as_of_date=as_of_date) @@ -462,12 +465,12 @@ def test_metastatic_as_of_date(self): # backdating consent beyond expired and the status lookup date # within a valid window should show available assessments. - # backdate so the baseline q's have expired - mock_qr(user_id=TEST_USER_ID, instrument_id='epic23', - status='in-progress', - timestamp=datetime.utcnow() - relativedelta(months=3)) self.bless_with_basics(backdate=relativedelta(months=3)) self.mark_metastatic() + # backdate so the baseline q's have expired + mock_qr(instrument_id='epic23', status='in-progress', + timestamp=datetime.utcnow() - relativedelta(months=3)) + self.test_user = db.session.merge(self.test_user) as_of_date = datetime.utcnow() - relativedelta(months=2, days=28) a_s = AssessmentStatus(user=self.test_user, as_of_date=as_of_date) @@ -553,9 +556,7 @@ def test_boundry_in_progress(self): self.bless_with_basics(backdate=relativedelta(months=3, hours=-1)) self.mark_localized() for instrument in localized_instruments: - mock_qr( - user_id=TEST_USER_ID, instrument_id=instrument, - status='in-progress') + mock_qr(instrument_id=instrument, status='in-progress') self.test_user = db.session.merge(self.test_user) a_s = AssessmentStatus(user=self.test_user, as_of_date=None) self.assertEquals(a_s.overall_status, 'In Progress') @@ -565,9 +566,7 @@ def test_boundry_in_progress_expired(self): self.bless_with_basics(backdate=relativedelta(months=3)) self.mark_localized() for instrument in localized_instruments: - mock_qr( - user_id=TEST_USER_ID, instrument_id=instrument, - status='in-progress') + mock_qr(instrument_id=instrument, status='in-progress') self.test_user = db.session.merge(self.test_user) a_s = AssessmentStatus(user=self.test_user, as_of_date=None) self.assertEquals(a_s.overall_status, 'Partially Completed') diff --git a/tests/test_communication.py b/tests/test_communication.py index 3c6e5a35a9..ae524e0fc5 100644 --- a/tests/test_communication.py +++ b/tests/test_communication.py @@ -161,12 +161,9 @@ def test_nearready_message(self): self.bless_with_basics(backdate=timedelta(days=13)) self.promote_user(role_name=ROLE.PATIENT) self.mark_localized() - mock_qr(user_id=TEST_USER_ID, instrument_id='eproms_add', - status='in-progress') - mock_qr(user_id=TEST_USER_ID, instrument_id='epic26', - status='in-progress') - mock_qr(user_id=TEST_USER_ID, instrument_id='comorb', - status='in-progress') + mock_qr(instrument_id='eproms_add', status='in-progress') + mock_qr(instrument_id='epic26', status='in-progress') + mock_qr(instrument_id='comorb', status='in-progress') update_patient_loop(update_cache=False, queue_messages=True) expected = Communication.query.first() @@ -184,12 +181,9 @@ def test_ready_message(self): self.bless_with_basics(backdate=timedelta(days=14)) self.promote_user(role_name=ROLE.PATIENT) self.mark_localized() - mock_qr(user_id=TEST_USER_ID, instrument_id='eproms_add', - status='in-progress') - mock_qr(user_id=TEST_USER_ID, instrument_id='epic26', - status='in-progress') - mock_qr(user_id=TEST_USER_ID, instrument_id='comorb', - status='in-progress') + mock_qr(instrument_id='eproms_add', status='in-progress') + mock_qr(instrument_id='epic26', status='in-progress') + mock_qr(instrument_id='comorb', status='in-progress') update_patient_loop(update_cache=False, queue_messages=True) expected = Communication.query.first() @@ -259,9 +253,9 @@ def test_done_message(self): self.bless_with_basics(backdate=timedelta(days=14)) self.promote_user(role_name=ROLE.PATIENT) self.mark_localized() - mock_qr(user_id=TEST_USER_ID, instrument_id='eproms_add') - mock_qr(user_id=TEST_USER_ID, instrument_id='epic26') - mock_qr(user_id=TEST_USER_ID, instrument_id='comorb') + mock_qr(instrument_id='eproms_add') + mock_qr(instrument_id='epic26') + mock_qr(instrument_id='comorb') update_patient_loop(update_cache=False, queue_messages=True) expected = Communication.query.first() @@ -350,7 +344,7 @@ def test_st_done(self): QuestionnaireBank.qbs_for_user(self.test_user, 'baseline')) for instrument in symptom_tracker_instruments: - mock_qr(user_id=TEST_USER_ID, instrument_id=instrument) + mock_qr(instrument_id=instrument) # With all q's done, shouldn't generate a message update_patient_loop(update_cache=False, queue_messages=True) @@ -371,7 +365,7 @@ def test_st_undone(self): QuestionnaireBank.qbs_for_user(self.test_user, 'baseline')) # With most q's undone, should generate a message - mock_qr(user_id=TEST_USER_ID, instrument_id='epic26') + mock_qr(instrument_id='epic26') a_s, _ = overall_assessment_status(TEST_USER_ID) self.assertEquals('In Progress', a_s) update_patient_loop(update_cache=False, queue_messages=True) @@ -395,7 +389,7 @@ def test_st_metastatic(self): QuestionnaireBank.qbs_for_user(self.test_user, 'baseline')) # shouldn't generate a message either - mock_qr(user_id=TEST_USER_ID, instrument_id='epic26') + mock_qr(instrument_id='epic26') update_patient_loop(update_cache=False, queue_messages=True) expected = Communication.query.first() self.assertFalse(expected) diff --git a/tests/test_intervention.py b/tests/test_intervention.py index 6ddc2e46af..3a21f8c27c 100644 --- a/tests/test_intervention.py +++ b/tests/test_intervention.py @@ -402,8 +402,8 @@ def test_card_html_update(self): dt = datetime(2017, 6, 10, 20, 00, 00, 000000) # Add a fake assessments and see a change for i in metastatic_baseline_instruments: - mock_qr(user_id=TEST_USER_ID, instrument_id=i, timestamp=dt) - mock_qr(user_id=TEST_USER_ID, instrument_id='irondemog', timestamp=dt) + mock_qr(instrument_id=i, timestamp=dt) + mock_qr(instrument_id='irondemog', timestamp=dt) user, ae = map(db.session.merge, (self.test_user, ae)) From fcb112c244d1f8df599993ef881d51ee2a7a417d Mon Sep 17 00:00:00 2001 From: OptimusRhine Date: Fri, 1 Dec 2017 17:04:45 -0800 Subject: [PATCH 03/19] adjusting test_intervention QNR test for indefinite QNR/QB --- tests/test_intervention.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_intervention.py b/tests/test_intervention.py index 3a21f8c27c..688b2ed452 100644 --- a/tests/test_intervention.py +++ b/tests/test_intervention.py @@ -16,6 +16,7 @@ from portal.models.intervention_strategies import AccessStrategy from portal.models.message import EmailMessage from portal.models.organization import Organization +from portal.models.questionnaire_bank import QuestionnaireBank from portal.models.role import ROLE from portal.models.user import add_role from portal.system_uri import DECISION_SUPPORT_GROUP, SNOMED @@ -403,7 +404,9 @@ def test_card_html_update(self): # Add a fake assessments and see a change for i in metastatic_baseline_instruments: mock_qr(instrument_id=i, timestamp=dt) - mock_qr(instrument_id='irondemog', timestamp=dt) + mi_qb = QuestionnaireBank.query.filter_by( + name='metastatic_indefinite').first() + mock_qr(instrument_id='irondemog', timestamp=dt, qb=mi_qb) user, ae = map(db.session.merge, (self.test_user, ae)) From 10a0eba7fe530d5c369d9ca7bdf37bdc650bc1ff Mon Sep 17 00:00:00 2001 From: Paul Bugni Date: Mon, 4 Dec 2017 13:59:58 -0800 Subject: [PATCH 04/19] Email reminders get dynamic lookup link `present_needed`. Fix for https://jira.movember.com/browse/TN-434 --- portal/models/communication.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/portal/models/communication.py b/portal/models/communication.py index fd289fccdd..ccf4e1c93a 100644 --- a/portal/models/communication.py +++ b/portal/models/communication.py @@ -8,7 +8,6 @@ from sqlalchemy.dialects.postgresql import ENUM from string import Formatter -from .assessment_status import AssessmentStatus # avoid cycle from .app_text import MailResource from ..audit import auditable_event from ..database import db @@ -38,16 +37,7 @@ def load_template_args(user, questionnaire_bank_id=None): """ def ae_link(): - now = datetime.utcnow() - assessment_status = AssessmentStatus(user=user, as_of_date=now) - link_url = url_for( - 'assessment_engine_api.present_assessment', - instrument_id=assessment_status. - instruments_needing_full_assessment(classification='all'), - resume_instrument_id=assessment_status. - instruments_in_progress(classification='all'), - _external=True) - return link_url + return url_for('assessment_engine_api.present_needed', _external=True) def make_button(text): inline = False From 4eb5742afd636ebdab109faef7f1fc7d5a0fa706 Mon Sep 17 00:00:00 2001 From: Paul Bugni Date: Tue, 5 Dec 2017 10:36:23 -0800 Subject: [PATCH 05/19] Adding function to retrieve document identifier (i.e. cPRO session id) from a QuestionnaireResponse by a set of query parameters. --- portal/models/fhir.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/portal/models/fhir.py b/portal/models/fhir.py index d500282936..a6714aec61 100644 --- a/portal/models/fhir.py +++ b/portal/models/fhir.py @@ -572,6 +572,30 @@ def aggregate_responses(instrument_ids, current_user): return bundle + +def qnr_document_id( + subject_id, questionniare_bank_id, questionnaire_name, status): + """Return document['identifier'] for matching QuestionnaireResponse + + Using the given filter data to look for a matching QuestionnaireResponse. + Expecting to find exactly one, or raises NoResultFound + + :return: the document identifier + + """ + qnr = QuestionnaireResponse.query.filter( + QuestionnaireResponse.status == status).filter( + QuestionnaireResponse.subject_id == subject_id).filter( + QuestionnaireResponse.document[ + ('questionnaire', 'reference') + ].astext.endswith(questionnaire_name)).filter( + QuestionnaireResponse.questionnaire_bank_id == + questionniare_bank_id).with_entities( + QuestionnaireResponse.document[( + 'questionnaire', 'identifier', 'value')]).one() + return qnr[0] + + def generate_qnr_csv(qnr_bundle): """Generate a CSV from a bundle of QuestionnaireResponses""" From 61dc906e844cc8c66694ada28d585d6fb3427fc3 Mon Sep 17 00:00:00 2001 From: Paul Bugni Date: Tue, 5 Dec 2017 10:39:18 -0800 Subject: [PATCH 06/19] Use dynamic lookup endpoint (and therefore consolidate the number of calculating endpoints) for outstanding assessments from strategy method. --- portal/models/intervention_strategies.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/portal/models/intervention_strategies.py b/portal/models/intervention_strategies.py index cdadbe7234..4982a53fb8 100644 --- a/portal/models/intervention_strategies.py +++ b/portal/models/intervention_strategies.py @@ -369,13 +369,7 @@ def completed_card_html(assessment_status): link_label = 'Continue questionnaire' if ( assessment_status.overall_status == 'In Progress') else ( 'Go to questionnaire') - link_url = url_for( - 'assessment_engine_api.present_assessment', - instrument_id=assessment_status. - instruments_needing_full_assessment(classification='all'), - resume_instrument_id=assessment_status. - instruments_in_progress(classification='all')) - + link_url = url_for('assessment_engine_api.present_needed') header = _(u"Open Questionnaire") card_html = u""" {intro} @@ -399,11 +393,7 @@ def completed_card_html(assessment_status): link_label = _(u'Continue questionnaire') if ( indefinite_questionnaires[1]) else ( _(u'Go to questionnaire')) - link_url = url_for( - 'assessment_engine_api.present_assessment', - instrument_id=indefinite_questionnaires[0], - resume_instrument_id=indefinite_questionnaires[1]) - + link_url = url_for('assessment_engine_api.present_needed') header = _(u"Open Questionnaire") card_html = u""" {intro} From d8c653a1bb7f4b0ec349039c2d96b2e604886788 Mon Sep 17 00:00:00 2001 From: Paul Bugni Date: Tue, 5 Dec 2017 10:40:34 -0800 Subject: [PATCH 07/19] Adding function (and tests) to retrieve document identifier (i.e. cPRO session id) from a QuestionnaireResponse by a set of query parameters. --- tests/test_assessment_status.py | 36 ++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/tests/test_assessment_status.py b/tests/test_assessment_status.py index bd4d27224d..e1e86a9595 100644 --- a/tests/test_assessment_status.py +++ b/tests/test_assessment_status.py @@ -2,13 +2,15 @@ from datetime import datetime from dateutil.relativedelta import relativedelta from flask_webtest import SessionScope +from random import randint +from sqlalchemy.orm.exc import NoResultFound from portal.extensions import db from portal.models.assessment_status import invalidate_assessment_status_cache from portal.models.assessment_status import AssessmentStatus from portal.models.audit import Audit from portal.models.encounter import Encounter -from portal.models.fhir import CC, QuestionnaireResponse +from portal.models.fhir import CC, QuestionnaireResponse, qnr_document_id from portal.models.intervention import INTERVENTION from portal.models.organization import Organization from portal.models.questionnaire import Questionnaire @@ -21,14 +23,23 @@ from tests import TestCase, TEST_USER_ID -def mock_qr(instrument_id, status='completed', timestamp=None, qb=None): +def mock_qr( + instrument_id, status='completed', timestamp=None, qb=None, + doc_id=None): + if not doc_id: + doc_id = randint(10000, 50000) timestamp = timestamp or datetime.utcnow() qr_document = { "questionnaire": { "display": "Additional questions", "reference": "https://{}/api/questionnaires/{}".format( - 'SERVER_NAME', instrument_id) + 'SERVER_NAME', instrument_id), + "identifier": { + "use": "official", + "label": "cPRO survey session ID", + "value": doc_id, + "system": "https://stg-ae.us.truenth.org/eproms-demo"} } } enc = Encounter(status='planned', auth_method='url_authenticated', @@ -301,6 +312,25 @@ def mark_metastatic(self): class TestAssessmentStatus(TestQuestionnaireSetup): + + def test_qnr_id(self): + qb = QuestionnaireBank.query.first() + mock_qr( + instrument_id='irondemog', + status='in-progress', qb=qb, + doc_id=21212) + qb = db.session.merge(qb) + result = qnr_document_id( + TEST_USER_ID, qb.id, 'irondemog', 'in-progress') + self.assertEquals(result, 21212) + + def test_qnr_id_missing(self): + qb = QuestionnaireBank.query.first() + qb = db.session.merge(qb) + with self.assertRaises(NoResultFound): + result = qnr_document_id( + TEST_USER_ID, qb.id, 'irondemog', 'in-progress') + def test_enrolled_in_metastatic(self): """metastatic should include baseline and indefinite""" self.bless_with_basics() From ab629f292a92fefd5cddee70114f04ee46ea39d4 Mon Sep 17 00:00:00 2001 From: Paul Bugni Date: Tue, 5 Dec 2017 10:43:17 -0800 Subject: [PATCH 08/19] Fix for TN-453 lookup and transmit the document identifier for any assessments `in-progress`. --- portal/views/assessment_engine.py | 36 ++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/portal/views/assessment_engine.py b/portal/views/assessment_engine.py index ec9896b88c..9cce7b2e91 100644 --- a/portal/views/assessment_engine.py +++ b/portal/views/assessment_engine.py @@ -16,7 +16,12 @@ from ..models.assessment_status import invalidate_assessment_status_cache from ..models.assessment_status import overall_assessment_status from ..models.auth import validate_origin -from ..models.fhir import QuestionnaireResponse, EC, aggregate_responses, generate_qnr_csv +from ..models.fhir import ( + aggregate_responses, + EC, + generate_qnr_csv, + QuestionnaireResponse, + qnr_document_id) from ..models.intervention import INTERVENTION from ..models.questionnaire import Questionnaire from ..models.questionnaire_bank import QuestionnaireBank @@ -1362,17 +1367,22 @@ def present_needed(): assessment_status.instruments_needing_full_assessment( classification='all')) - # As the AssessmentEngine isn't yet equipped to restart out - # of sequence instruments, treat all as new if as_of_date - # isn't today. - if as_of_date and as_of_date.date() != datetime.utcnow().date(): - args['instrument_id'] += ( - assessment_status.instruments_in_progress( - classification='all')) - else: - args['resume_instrument_id'] = ( - assessment_status.instruments_in_progress( - classification='all')) + # If we find any instruments_in_progress, need to fetch their + # identifiers for reliable resume behavior on the AE side. + # This is also done now to avoid the overhead of looking up + # when generating reports and reminders. + resume_ids = [] + for questionnaire_name in assessment_status.instruments_in_progress( + classification=all): + resume_ids.append( + qnr_document_id( + subject_id=subject_id, + questionniare_bank_id=assessment_status.qb_data.qb.id, + questionnaire_name=questionnaire_name, + status='in-progress')) + + if resume_ids: + args['resume_identifier'] = resume_ids url = url_for('.present_assessment', **args) return redirect(url, code=303) @@ -1453,6 +1463,7 @@ def present_assessment(instruments=None): queued_instruments = request.args.getlist('instrument_id') resume_instruments = request.args.getlist('resume_instrument_id') + resume_identifiers = request.args.getlist('resume_identifier') # Hack to allow deprecated API to piggyback # Remove when deprecated_present_assessment() is fully removed @@ -1477,6 +1488,7 @@ def present_assessment(instruments=None): assessment_params = { "project": ",".join(common_instruments), "resume_instrument_id": ",".join(resume_instruments), + "resume_identifier": ",".join(resume_identifiers), "subject_id": request.args.get('subject_id'), "authored": request.args.get('authored'), } From 26e261ce4e296fa3ac92f84b07fbc1d48379c67a Mon Sep 17 00:00:00 2001 From: Paul Bugni Date: Tue, 5 Dec 2017 12:54:27 -0800 Subject: [PATCH 09/19] Use random strings as document identifiers to match expectations --- tests/test_assessment_status.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/test_assessment_status.py b/tests/test_assessment_status.py index e1e86a9595..04381bb5b6 100644 --- a/tests/test_assessment_status.py +++ b/tests/test_assessment_status.py @@ -2,8 +2,9 @@ from datetime import datetime from dateutil.relativedelta import relativedelta from flask_webtest import SessionScope -from random import randint +from random import choice from sqlalchemy.orm.exc import NoResultFound +from string import ascii_letters from portal.extensions import db from portal.models.assessment_status import invalidate_assessment_status_cache @@ -27,7 +28,7 @@ def mock_qr( instrument_id, status='completed', timestamp=None, qb=None, doc_id=None): if not doc_id: - doc_id = randint(10000, 50000) + doc_id = ''.join(choice(ascii_letters) for _ in range(10)) timestamp = timestamp or datetime.utcnow() qr_document = { "questionnaire": { @@ -318,11 +319,11 @@ def test_qnr_id(self): mock_qr( instrument_id='irondemog', status='in-progress', qb=qb, - doc_id=21212) + doc_id='two11') qb = db.session.merge(qb) result = qnr_document_id( TEST_USER_ID, qb.id, 'irondemog', 'in-progress') - self.assertEquals(result, 21212) + self.assertEquals(result, 'two11') def test_qnr_id_missing(self): qb = QuestionnaireBank.query.first() From e1eabc0403171da301f19940004560946ce34fe6 Mon Sep 17 00:00:00 2001 From: Paul Bugni Date: Tue, 5 Dec 2017 15:44:08 -0800 Subject: [PATCH 10/19] Only attempt to parse datetime if provided. --- portal/views/assessment_engine.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/portal/views/assessment_engine.py b/portal/views/assessment_engine.py index 9cce7b2e91..8a487afaca 100644 --- a/portal/views/assessment_engine.py +++ b/portal/views/assessment_engine.py @@ -1360,7 +1360,8 @@ def present_needed(): if subject != current_user(): current_user().check_role(permission='edit', other_id=subject_id) - as_of_date = FHIR_datetime.parse(request.args.get('authored')) + authored = request.args.get('authored') + as_of_date = FHIR_datetime.parse(authored) if authored else None assessment_status = AssessmentStatus(subject, as_of_date=as_of_date) args = dict(request.args.items()) args['instrument_id'] = ( From c5b73014674bccd1108e5e57a51d9ae376485c98 Mon Sep 17 00:00:00 2001 From: Paul Bugni Date: Tue, 5 Dec 2017 18:20:01 -0800 Subject: [PATCH 11/19] Bug fixes - need the string 'all' (not the python builtin). Correct path w/i json questionnaire response documents for the identifier. --- portal/models/fhir.py | 8 ++++---- portal/views/assessment_engine.py | 4 ++-- tests/test_assessment_status.py | 14 +++++++------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/portal/models/fhir.py b/portal/models/fhir.py index a6714aec61..56daf0d14e 100644 --- a/portal/models/fhir.py +++ b/portal/models/fhir.py @@ -574,13 +574,13 @@ def aggregate_responses(instrument_ids, current_user): def qnr_document_id( - subject_id, questionniare_bank_id, questionnaire_name, status): + subject_id, questionnaire_bank_id, questionnaire_name, status): """Return document['identifier'] for matching QuestionnaireResponse Using the given filter data to look for a matching QuestionnaireResponse. Expecting to find exactly one, or raises NoResultFound - :return: the document identifier + :return: the document identifier value, typically a string """ qnr = QuestionnaireResponse.query.filter( @@ -590,9 +590,9 @@ def qnr_document_id( ('questionnaire', 'reference') ].astext.endswith(questionnaire_name)).filter( QuestionnaireResponse.questionnaire_bank_id == - questionniare_bank_id).with_entities( + questionnaire_bank_id).with_entities( QuestionnaireResponse.document[( - 'questionnaire', 'identifier', 'value')]).one() + 'identifier', 'value')]).one() return qnr[0] diff --git a/portal/views/assessment_engine.py b/portal/views/assessment_engine.py index 8a487afaca..f07d8a154d 100644 --- a/portal/views/assessment_engine.py +++ b/portal/views/assessment_engine.py @@ -1374,11 +1374,11 @@ def present_needed(): # when generating reports and reminders. resume_ids = [] for questionnaire_name in assessment_status.instruments_in_progress( - classification=all): + classification='all'): resume_ids.append( qnr_document_id( subject_id=subject_id, - questionniare_bank_id=assessment_status.qb_data.qb.id, + questionnaire_bank_id=assessment_status.qb_data.qb.id, questionnaire_name=questionnaire_name, status='in-progress')) diff --git a/tests/test_assessment_status.py b/tests/test_assessment_status.py index 04381bb5b6..e12b34d960 100644 --- a/tests/test_assessment_status.py +++ b/tests/test_assessment_status.py @@ -35,14 +35,14 @@ def mock_qr( "display": "Additional questions", "reference": "https://{}/api/questionnaires/{}".format( - 'SERVER_NAME', instrument_id), - "identifier": { - "use": "official", - "label": "cPRO survey session ID", - "value": doc_id, - "system": "https://stg-ae.us.truenth.org/eproms-demo"} - } + 'SERVER_NAME', instrument_id)}, + "identifier": { + "use": "official", + "label": "cPRO survey session ID", + "value": doc_id, + "system": "https://stg-ae.us.truenth.org/eproms-demo"} } + enc = Encounter(status='planned', auth_method='url_authenticated', user_id=TEST_USER_ID, start_time=timestamp) with SessionScope(db): From 633fe53a074c9ee5291ece14719a8c0ec03d91ab Mon Sep 17 00:00:00 2001 From: Paul Bugni Date: Wed, 6 Dec 2017 12:01:35 -0800 Subject: [PATCH 12/19] Adding a missing test for multiple QB logic. --- tests/test_assessment_status.py | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/tests/test_assessment_status.py b/tests/test_assessment_status.py index e12b34d960..3aee34da5b 100644 --- a/tests/test_assessment_status.py +++ b/tests/test_assessment_status.py @@ -528,10 +528,41 @@ def test_initial_recur_due(self): set(a_s.instruments_needing_full_assessment()), metastatic_3) + def test_initial_recur_baseline_done(self): + # backdate to be within the first recurrence window + + self.bless_with_basics(backdate=relativedelta(months=3, days=2)) + self.mark_metastatic() + + # add baseline QNRs, as if submitted nearly 3 months ago, during + # baseline window + backdated = datetime.utcnow() - relativedelta(months=2, days=25) + baseline = QuestionnaireBank.query.filter_by( + name='metastatic').one() + for instrument in metastatic_baseline_instruments: + mock_qr(instrument, qb=baseline, timestamp=backdated) + + self.test_user = db.session.merge(self.test_user) + # Check status during baseline window + a_s_baseline = AssessmentStatus( + user=self.test_user, as_of_date=backdated) + self.assertEquals(a_s_baseline.overall_status, "Completed") + self.assertFalse(a_s_baseline.instruments_needing_full_assessment()) + + # Whereas "current" status for the initial recurrence show due. + a_s = AssessmentStatus(user=self.test_user, as_of_date=None) + self.assertEquals(a_s.overall_status, "Due") + + # in the initial window w/ no questionnaires submitted + # should include all from initial recur + self.assertEquals( + set(a_s.instruments_needing_full_assessment()), + metastatic_3) + def test_secondary_recur_due(self): # backdate so baseline q's have expired, and we are within the - # second recurrance window + # second recurrence window self.bless_with_basics(backdate=relativedelta(months=6)) self.mark_metastatic() self.test_user = db.session.merge(self.test_user) From 25fbac30b390c90651425f08422c8f24eac994de Mon Sep 17 00:00:00 2001 From: Paul Bugni Date: Wed, 6 Dec 2017 12:03:16 -0800 Subject: [PATCH 13/19] Need special handling for the `indefinite` questionnaire banks, so users may enter manually even if all other questionnaires have been completed. --- portal/templates/profile_macros.html | 5 ++--- portal/views/assessment_engine.py | 12 +++++++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/portal/templates/profile_macros.html b/portal/templates/profile_macros.html index 56def2bada..3a0b617f07 100644 --- a/portal/templates/profile_macros.html +++ b/portal/templates/profile_macros.html @@ -2091,8 +2091,6 @@