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

Atlas Cohort Definition Validation Feature #2593

Draft
wants to merge 50 commits into
base: master
Choose a base branch
from

Conversation

charhart
Copy link

@charhart charhart commented Aug 22, 2021

This pull request creates the Atlas features for cohort definition validation. There are primarily three functional areas impacted by this new feature. This pull request corresponds to this WebAPI pull request.

  1. Cohort Definition
  • On the Samples tab, provides an option to create a validation study, which essentially links the sample to the validation question set.
  • Creates the new 'Validation' tab, which allows for the creation of question sets, and the linking of question sets to sample in order to create studies.
  1. Profile
  • Adds a UI feature to annotate validation studies, which appears on the right of the page
  • Adds optional routing parameters that link explicitly to samples and question sets (the validation study).

#2491 describes this feature in greater detail

@jduke99 can provide additional details.

charhart and others added 30 commits February 1, 2021 11:25
@charhart
Copy link
Author

I have some additional tweaks to make on my end after the WebAPI PR was updated. Thanks!

@charhart
Copy link
Author

Okay, I'm done on my end. @jduke99 is still reviewing on our end.

@chrisknoll
Copy link
Collaborator

Charity, I noticed that this PR is comming off of a branch from your master branch in your fork, which is making it a little awkward. It makes getting upstream fetches from OHDSI/Atlas master merged into your fork's master branch which contains both your new functionality and upstream commits. Can you isolate your new feature (annotation) into a dedicated branch in your fork?

@chrisknoll
Copy link
Collaborator

I'm getting an error when trying to create/modify a question set.:
image

In this image, I'm seeing a console error about how 'type' is not a function. when looking at how a Question is created, I do see that the type property of Question is set to ko.observable(''). So, i believe that would be the third element in the questions() collection. Somewhere, the type property of the object is losing the observable status and becomming the raw type...possibly after the question set is saved...I'm not sure yet, but there is an error here.

@chrisknoll
Copy link
Collaborator

Looking further into the implementation, I think the problem stems from QuestionSetForm.js, around lines 128-151:

            for (i = 0; i < self.questions().length; i++) {
                if (self.questions()[i].type() === 'Text') {
                    self.questions()[i].answers.push(new Answer());
                    self.questions()[i].answers()[0].text = '';
                    self.questions()[i].type = 'TEXTAREA';
                } else if (self.questions()[i].type() === 'Numeric') {
                    self.questions()[i].answers.push(new Answer());
                    self.questions()[i].answers()[0].text = '';
                    self.questions()[i].type = 'NUMERIC';
                } else if (self.questions()[i].type() === 'Date') {
                    self.questions()[i].answers.push(new Answer());
                    self.questions()[i].answers()[0].text = '';
                    self.questions()[i].type = 'DATE';
                } else if (self.questions()[i].type() === "Checkbox") {
                    self.questions()[i].type = 'MULTI_SELECT';
                    for (j = 0; j < self.questions()[i].answers().length; j++) {
                        currentAnswer = self.questions()[i].answers()[j];
                        currentAnswer.value = currentAnswer.text();
                    }
                } else if (self.questions()[i].type() === "Radio Button") {
                    self.questions()[i].type = 'SINGLE_SELECT';
                    for (j = 0; j < self.questions()[i].answers().length; j++) {
                        currentAnswer = self.questions()[i].answers()[j];
                        currentAnswer.value = j;
                    }
                }
            }

If you notice, each of the else clauses assigns the type property back to a native type, and not an observable value:

                    self.questions()[i].type = 'TEXTAREA';
...
                    self.questions()[i].type = 'NUMERIC';
,,,
                    self.questions()[i].type = 'DATE';

Once that happens, you can't call questions()[i].type() anymore (since it's not an observable). So I think that is where the data corruption is coming from (corruption = observable converted to non-observable).

Perhaps part of an issue at play here is a misunderstanding between a 'viewmodel' and the raw data, but we have a few places in code that could serve as an example about how you can define a JavaScript object to represent the object as a viewmodel. It looks like Question and Answer have an implementation:

        function Question() {
            const self = this;
            self.text = ko.observable('');
            // self.helpText = ko.observable('');
            self.type = ko.observable('');
            self.caseQuestion = ko.observable('');
            self.required = ko.observable('');
            self.answers = ko.observableArray([]);

            self.addAnswer = function () {
                if ((self.type() !== 'Text' && self.type() !== 'Numeric' && self.type() !== 'Date') && self.type() !== undefined) {
                    self.answers.push(new Answer());
                }
            };
            self.removeAnswer = function (item) {
                self.answers.remove(item);
            };

            self.questionTypeChanged = function (obj, evt) {
                if (obj !== 'Text' && obj !== 'Numeric' && obj !== 'Date') {
                    if (self.answers().length === 0) {
                        self.addAnswer();
                    }
                }
            };
        }

However, I would have put this into a separate file for annotation 'types', and passed a constructor object into the function (ie: new Answer(data) {...}) such that the answer object can be initialized based on some other JSON/JSObject source (such as something that comes back from a web service endpiont).

Examples of where we create viewmodels to represent data objects:
https://github.com/OHDSI/Atlas/blob/master/js/components/cohortbuilder/InputTypes/Occurrence.js
https://github.com/OHDSI/Atlas/blob/master/js/components/cohortbuilder/InputTypes/Text.js

Etc. I use these as examples because these elements are the ones used to capture user input for cohort definitions, and so it might be a good guide on creating objects meant to capture user input about question set definitions.

Updates for cohort annotation PR
@anthonysena anthonysena linked an issue Feb 8, 2022 that may be closed by this pull request
@chrisknoll chrisknoll self-assigned this Feb 21, 2022
@anthonysena anthonysena marked this pull request as draft June 13, 2023 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Atlas Cohort Definition Validation Function
3 participants