Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[New Feature] Tracking and Configuration of Candidate's Diagnosis Evolution #7560

Merged
merged 15 commits into from
Jan 11, 2024

Conversation

jesscall
Copy link
Contributor

@jesscall jesscall commented Aug 31, 2021

Brief summary of changes

This PR adds a way for project administrators to define and configure diagnosis trajectories for their project. The aim of this new feature is multifaceted; it allows study coordinators to

  1. Track the evolution of a patient's diagnosis over time
  2. Determine (& query) the candidate's latest diagnosis is without having to manually compare the longitudinal data

Interest for this feature was expressed by multiple projects during the DCC meeting on July 7th.


What is included in this PR?

1. A configuration page for diagnosis evolution

Screen Shot 2021-08-31 at 2 50 15 PM

2. A tab in candidate parameters to view details on the evolution of the diagnosis.

Screen Shot 2021-08-31 at 2 51 17 PM

3. Logic to propagate the latest diagnosis data as instruments are updated.
4. A script that all projects should run when configuring Dx Trajectories and updating their instances.
5. Updates the demographic import to couchDB so the latest diagnosis can be queried.


Info

A Diagnosis Trajectory is defined by a source field, or group of source fields, that are found within the test battery.
A patient's Diagnosis Evolution is defined by the progression of changing diagnoses over time. The evolution is determined by the order of configured Diagnosis Trajectories.

There are two ways that the Latest Diagnosis of the candidate can be updated. First, the script in this PR can be run. Secondly, when ANY instrument part of a configured diagnosis track is saved, the latest diagnosis is determined and updated in the candidate table.

EX:

Screen Shot 2021-08-31 at 3 29 58 PM

Screen Shot 2021-08-31 at 3 16 59 PM

  • Have you updated related documentation?

Testing instructions (if applicable)

  1. fetch jesscall && git checkout jesscall/2021_07_27_diagnosis_evolution
  2. cat SQL/New_patches/2021-07-28_diagnosis_evolution.sql | mysql
  3. make dev
  4. Follow instructions in configuration & caniddate parameters TestPlan.md files included in this PR
  5. After setting up the configuration for a diagnosis evolution please do the following:
  6. php tools/data_integrity/update_candidate_latest_diagnosis.php
  7. Ensure that updating data in a configured instrument updates the latest diagnosis in the candidate table (if it's the latest) and the candidate parameters tab.

Link(s) to related PR(s)

@jesscall jesscall added Category: Feature PR or issue that aims to introduce a new feature Release: Breaking changes PR that contains changes that might impact the code or accepted practices of active projects Language: SQL PR or issue that update SQL code State: Blocked PR or issue awaiting an external event such as the merge or another PR to proceed Language: PHP PR or issue that update PHP code labels Aug 31, 2021
@zaliqarosli zaliqarosli self-requested a review September 14, 2021 21:49
Copy link
Contributor

@zaliqarosli zaliqarosli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is really cool! setting it all up works well. i have a couple bugs, comments and a request.

some bugs:

  • if I click on the trajectory tab I just created, then back to 'New Diagnosis Trajectory' tab, the form doesn't re-render as a new form, it still shows the trajectory I just created

Screen Shot 2021-09-30 at 2 44 22 PM

  • reset button doesn't work

request: is there a possibility to add a timestamp?

questions: is candidate_latest_diagnosis really just candidate_diagnosis_rel? i.e. do all the entries in the Diagnosis Evolution table on the front-end get stored in there?

Copy link
Contributor

@zaliqarosli zaliqarosli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 more bug, the second field from the group of source fields is not getting saved in the LatestDiagnosis column
Screen Shot 2021-09-30 at 4 48 07 PM

SQL/New_patches/2021-07-28_diagnosis_evolution.sql Outdated Show resolved Hide resolved
@jesscall
Copy link
Contributor Author

jesscall commented Oct 5, 2021

Hey @zaliqarosli
I'm working on reactifying this PR as well as adding a proper endpoint as per @driusan request in a private convo.

I'll try to get to your feedbacks afterwards and see what is still relevant.

As for the rendering of diagnosis for QPN, that's a tough one. I imagine they are dropdown options? That sort of information isn't captured by data dictionary / parameter type. I guess you could hard-code the mapping or use grep on the instrument linst file... But these are both pretty hacky solutions. Maybe @ridz1208 has a suggestion

@jesscall jesscall added the State: Needs work PR awaiting additional work by the author to proceed label Oct 5, 2021
@jesscall jesscall force-pushed the 2021_07_27_diagnosis_evolution branch from e68b0e0 to 35fea1d Compare October 5, 2021 20:08
@jesscall
Copy link
Contributor Author

jesscall commented Oct 5, 2021

@zaliqarosli
I've updated the PR. It is now reactified, using a php endpoint rather than ajax and I've also addressed your comments :)
Feel free to re-review at your convenience

@driusan your requested changes have been implemented

@jesscall jesscall removed the State: Needs work PR awaiting additional work by the author to proceed label Oct 5, 2021
Copy link
Contributor

@zaliqarosli zaliqarosli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screen Shot 2021-10-05 at 6 50 24 PM

should the Source Field be the database field name without the instrument prepended to it? i believe that was how it was implemented before reactification

@jesscall
Copy link
Contributor Author

jesscall commented Oct 6, 2021

Screen Shot 2021-10-05 at 6 50 24 PM

should the Source Field be the database field name without the instrument prepended to it? i believe that was how it was implemented before reactification

@zaliqarosli

So in switching from templates to React, the datalist is implemented differently with our predefined react components. Before, you could search by the field name or the table name because both values were displayed in the select option. With the react component you can only have one of the two. So either:

  1. The field name alone - EX: weight_lbs (parameter_type -> SourceField)
  2. The compounded table name and field name - EX: bmi_weight_lbs (parameter_type -> Name)

I opted for (2) to make searching by instrument an option for the user. If you want this changed, let me know.

@jesscall
Copy link
Contributor Author

jesscall commented Oct 25, 2021

Latest Changes:

  • Added timestamp to track last update of latest diagnosis
  • reverted back to using the sourceField names that are not prepended with instrument name.

Screen Shot 2021-10-25 at 2 02 48 PM

Screen Shot 2021-10-25 at 2 02 40 PM

@jesscall
Copy link
Contributor Author

jesscall commented Nov 1, 2021

Refactored this PR to track the diagnosis evolution as a history rather than the current latest diagnosis. Also implemented @ridz1208 suggestion for showing the confirmation status of a diagnosis (whether the diagnosis comes from an approved visit or not).

Screen Shot 2021-10-28 at 3 22 20 PM


Screen Shot 2021-10-28 at 3 17 45 PM

@ridz1208 ridz1208 removed their request for review January 10, 2022 18:12
@driusan
Copy link
Collaborator

driusan commented Jun 6, 2022

@jesscall what is the status of this?

@driusan
Copy link
Collaborator

driusan commented Nov 21, 2023

I'm not sure who's maintaining this PR now but the tests on it are failing and there's a few eslint warnings that need to be fixed (the error ones are the reason it's failing)

/home/runner/work/Loris/Loris/modules/configuration/jsx/DiagnosisEvolution.js
  125:1   error    The type 'int' is undefined                                           jsdoc/no-undefined-types
  303:1   error    The type 'bool' is undefined                                          jsdoc/no-undefined-types
  399:13  warning  Unexpected console statement                                          no-console
  494:13  warning  Unexpected console statement                                          no-console
  504:1   warning  @param "id" does not match an existing function parameter             jsdoc/check-param-names
  548:1   warning  @param "pendingValKey" does not match an existing function parameter  jsdoc/check-param-names

@zaliqarosli
Copy link
Contributor

@driusan ya im working on it

@driusan driusan assigned driusan and unassigned zaliqarosli Dec 5, 2023
@driusan
Copy link
Collaborator

driusan commented Dec 6, 2023

I am getting 2 errors and a blank page on the configuration -> Diagnosis Evolution page:

Warning: ReactDOM.render is no longer supported in React 18. Use createRoot instead. Until you switch to the new API, your app will behave as if it's running React 17. Learn more: https://reactjs.org/link/switch-to-createroot [react-dom.development.js:10:1247](http://localhost:8000/vendor/js/react/react-dom.development.js)
Uncaught ReferenceError: FormElement is not defined
    renderDiagnosisForm DiagnosisEvolution.js:158
    render DiagnosisEvolution.js:291
    React 16
    componentDidMount DiagnosisEvolution.js:54
    promise callback*componentDidMount DiagnosisEvolution.js:54
    React 12
    <anonymous> DiagnosisEvolution.js:582
    EventListener.handleEvent* DiagnosisEvolution.js:581
    <anonymous> DiagnosisEvolution.js:6
    <anonymous> DiagnosisEvolution.js:6
[DiagnosisEvolution.js:158:20](webpack://lorisjs.configuration/modules/configuration/jsx/DiagnosisEvolution.js)
    renderDiagnosisForm DiagnosisEvolution.js:158
    render DiagnosisEvolution.js:291
    React 11
    performSyncWorkOnRoot self-hosted:1406
    React 5
    componentDidMount DiagnosisEvolution.js:54
    (Async: promise callback)
    componentDidMount DiagnosisEvolution.js:54
    React 7
    performSyncWorkOnRoot self-hosted:1406
    React 5
    <anonymous> DiagnosisEvolution.js:582
    (Async: EventListener.handleEvent)
    <anonymous> DiagnosisEvolution.js:581
    <anonymous> DiagnosisEvolution.js:6
    <anonymous> DiagnosisEvolution.js:6

The first seems to be because something is written using an old/deprecated style from react. The second is because import statements from jsx/Form are missing (I'm getting similar errors on the candidate_parameters page but I'll fix those on the other tabs and send a PR since I think that's broken on main.)

@driusan
Copy link
Collaborator

driusan commented Dec 6, 2023

Actually it looks like the imports are already there on main. The branch needs to be rebased so that the candidate_parameters loads (and the new tab may need to be fixed.)

jesscall and others added 13 commits December 6, 2023 20:04
author jesscall <[email protected]> 1627406855 -0400
committer zaliqarosli <[email protected]> 1699561499 -0500

parent ad0460c
author jesscall <[email protected]> 1627406855 -0400
committer zaliqarosli <[email protected]> 1699561457 -0500

parent ad0460c
author jesscall <[email protected]> 1627406855 -0400
committer zaliqarosli <[email protected]> 1699561433 -0500

parent ad0460c
author jesscall <[email protected]> 1627406855 -0400
committer zaliqarosli <[email protected]> 1699561346 -0500

parent ad0460c
author jesscall <[email protected]> 1627406855 -0400
committer zaliqarosli <[email protected]> 1699561321 -0500

parent ad0460c
author jesscall <[email protected]> 1627406855 -0400
committer zaliqarosli <[email protected]> 1699561301 -0500

parent ad0460c
author jesscall <[email protected]> 1627406855 -0400
committer zaliqarosli <[email protected]> 1699561256 -0500

parent ad0460c
author jesscall <[email protected]> 1627406855 -0400
committer zaliqarosli <[email protected]> 1699560890 -0500

[Diagnosis Evolution] First commit

[Diagnosis Evolution] Validation & progress

start cand param

cand param tab

more progress, getLatestDx, multiple sourceFields

switching to propagate data - not done

propagate data done, caveat script done

schema

test plans

more documentation

Added LatestDX to CouchDB Demographics import

bug fix on candidate get latest source

phpcs

fixing errors

added project to config and cand param, need to revamp sql structure

progress on the sql structure and reworking logic

multi-project support implemented

Amend CouchDB Demographics Import for multiproject support of latest Dx

phpcs

progress on reactification

progress

reactification & php endpoint

cleanup

addressing zal's comments

align buttons

timestamp

reverted back to sourcefield from parameter type

phan

starting refactor

revamped react, backend refactored to dx history and confrimed

logic completely reworked, front end finished. Need to update documentation and DQT imports

add configured order to cand param

couchDB demographics import updated

phpcs

propagate logic fix

utility fxn to update confrimation status

some lil fixes

changes requested & changing confirmed = passed visit

changing db initialization

phan

Apply zal's suggestions from code review

Co-authored-by: Zaliqa <[email protected]>

Apply zal's suggestions to schema

amend param comment

Update php/libraries/Utility.class.inc

Update php/libraries/NDB_BVL_Instrument.class.inc

Update modules/configuration/php/diagnosis.class.inc

Update modules/candidate_parameters/ajax/getData.php

Update SQL/New_patches/2021-07-28_diagnosis_evolution.sql

Update SQL/0000-00-00-schema.sql

rm links to other pages

Co-authored-by: Zaliqa <[email protected]>

Apply suggestions from code review

Co-authored-by: Zaliqa <[email protected]>

Update modules/configuration/php/diagnosis.class.inc

travis

update diagnosis if at least 1 field has data

Co-authored-by: Zaliqa <[email protected]>

zal's comment about pselect

dont duplicate same diagnosis if confirmed

Update tools/data_integrity/update_candidate_latest_diagnosis.php

Co-authored-by: Zaliqa <[email protected]>

lintphp

fix null checks

add approval status check

cleanup and fix reset

restrict saving diagnosis evolution for cands projects

check name conflict as case insensitive

Update tools/CouchDB_Import_Demographics.php

To avoid error with projects that have a space in their name

Co-authored-by: CamilleBeau <[email protected]>

Update tools/CouchDB_Import_Demographics.php

To avoid error with projects that have a space in their name

Co-authored-by: CamilleBeau <[email protected]>

Update modules/configuration/php/diagnosis.class.inc

This caused a bug on CCNA when trying to set diagnosis evolution if the visits haven't yet been started, so I did a select from test_battery instead of session

Co-authored-by: CamilleBeau <[email protected]>

Update modules/configuration/php/diagnosis.class.inc

Co-authored-by: CamilleBeau <[email protected]>

cleanup

declare DB

cleanup repeat declaration

add Delete button

[Diagnosis Evolution] First commit

[Diagnosis Evolution] Validation & progress

cand param tab

more progress, getLatestDx, multiple sourceFields

schema

more documentation

Added LatestDX to CouchDB Demographics import

phpcs

added project to config and cand param, need to revamp sql structure

multi-project support implemented

Amend CouchDB Demographics Import for multiproject support of latest Dx

phpcs

progress on reactification

progress

reactification & php endpoint

cleanup

addressing zal's comments

align buttons

timestamp

reverted back to sourcefield from parameter type

starting refactor

logic completely reworked, front end finished. Need to update documentation and DQT imports

propagate logic fix

some lil fixes

changes requested & changing confirmed = passed visit

Apply zal's suggestions to schema

amend param comment

Update php/libraries/Utility.class.inc

Update php/libraries/NDB_BVL_Instrument.class.inc

Update SQL/New_patches/2021-07-28_diagnosis_evolution.sql

Update SQL/0000-00-00-schema.sql

rm links to other pages

Co-authored-by: Zaliqa <[email protected]>

Update modules/configuration/php/diagnosis.class.inc

travis

zal's comment about pselect

dont duplicate same diagnosis if confirmed

Update tools/data_integrity/update_candidate_latest_diagnosis.php

Co-authored-by: Zaliqa <[email protected]>

lintphp

fix null checks

add approval status check

restrict saving diagnosis evolution for cands projects

check name conflict as case insensitive

Update tools/CouchDB_Import_Demographics.php

To avoid error with projects that have a space in their name

Co-authored-by: CamilleBeau <[email protected]>

Update modules/configuration/php/diagnosis.class.inc

Co-authored-by: CamilleBeau <[email protected]>

declare DB

cleanup repeat declaration

phpcs

Test suite changes

fix action declaration

whitespace

Update modules/configuration/php/diagnosis.class.inc

Update modules/configuration/test/TestPlan.md

Co-authored-by: Zaliqa <[email protected]>

Update modules/candidate_parameters/test/TestPlan.md

Co-authored-by: Zaliqa <[email protected]>

Fix Delete & testplan

Cleanup, required elenents error, clearing form

phpcs

typo

fix project checking in script

phpcs

Fix resetting of old diagnosis trajectories

Add SubmitURL

SubmitUrl proptype
@zaliqarosli
Copy link
Contributor

@driusan fixed, and checks passed. could you have another look?
@CamilleBeau would you be able to do a final quick run through test? i had to make some extra changes after the rebase (see last 4 commits)

Copy link
Collaborator

@driusan driusan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty cool, just some small comments.

I also find the "SearchableDropdown" on the configuration page confusing because it looks like a text field.

SQL/0000-00-00-schema.sql Outdated Show resolved Hide resolved
SQL/0000-00-00-schema.sql Outdated Show resolved Hide resolved
<p>This diagnosis is <strong style={{color: 'green'}}>
confirmed</strong>.
A confirmed diagnosis is one that belongs to a visit
that has been sent to DCC for approval.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this criteria. Why does sending to DCC for approval mean the diagnosis is confirmed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's the workflow that was agreed on during a loris meeting, and what was implemented by CCNA. sending a timepoint to DCC makes the diagnosis for that configured visit confirmed, before that the diagnosis isn't confirmed

modules/configuration/php/diagnosis.class.inc Show resolved Hide resolved
@zaliqarosli
Copy link
Contributor

@driusan the SearchableDropdown being used is just the one that is in the jsx form elements file so its UI can be targeted in a separate if necessary. I added the FK for test name, ready for your review/testing again

@driusan driusan merged commit a0a8187 into aces:main Jan 11, 2024
19 checks passed
@ridz1208 ridz1208 added this to the 26.0.0 milestone Apr 9, 2024
github-merge-queue bot pushed a commit that referenced this pull request Sep 27, 2024
After We merge the diagnosis new feature into 26.0-release([New Feature]
Tracking and Configuration of Candidate's Diagnosis Evolution
(#7560)), it cause a bug.
if diagnosis is empty. the whole candidate param page will be broken.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Feature PR or issue that aims to introduce a new feature Language: PHP PR or issue that update PHP code Language: SQL PR or issue that update SQL code Passed manual tests PR has been successfully tested by at least one peer Release: Breaking changes PR that contains changes that might impact the code or accepted practices of active projects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants