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

Query DSL replacing EligibilityModel workflow #312

Merged
merged 19 commits into from
Aug 8, 2019

Conversation

Datamance
Copy link
Contributor

WIP, fixes #141

Trying out filtrex to parse boolean algebras, which would be a possible
addendum to (or replacement for) the simpler require/exclude mechanisms
currently in place for characteristics/conditions and languages.

Also made some simple language changes.
We can now do AND and OR queries on Bitfields using a special manager
that I have created, with two methods: has_one_of and has_all_of.
This is a work in progress - remaining todos include:

1) Integrating with view logic
2) Redesigning eligibility view
3) Further specifying grammar to handle gender specifications more
naturally, as well as better handling of gestational age.
Here we are, wired up all the way through the frontend!

TODO:

1) deprecate WIP eligibility view. I'm sure we'll be able to find a
use for some of that code later :)

2) Modify docs to house query DSL and provide a link inline with the
study edit and create forms.
In Study Edit and Study Create forms.
These should be bulked up a bit - in particular the complex condition.
Compound OR and AND expressions allowed.
Tests included.
@Datamance Datamance added Researcher [Audience] Researcher-facing researcher usability labels Aug 7, 2019
@Datamance Datamance added this to the Block 2 milestone Aug 7, 2019
@Datamance Datamance self-assigned this Aug 7, 2019
@Datamance Datamance added this to In progress in Lookit via automation Aug 7, 2019
@kimberscott
Copy link
Contributor

I see we're checking the criteria string to check that the form is as expected (so that I get e.g. Invalid criteria expression: No terminal defined for 'X' at line 1 col 28 deaf AND has_too_many_cats XOR is_silly ^ Expecting: {'OR', 'GT', 'AND', 'LT', 'LTE', 'NE', 'GTE', 'EQ'} ). How complicated would it be to also check the tokens are valid, so that typos don't lead to unpredicted behavior?

For instance I can currently set eligibility to deaf AND has_too_many_cats without error - upon trying to participate everyone's rejected (although deaf OR has_too_many_cats lets deaf kids participate)

accounts/queries.py Outdated Show resolved Hide resolved
accounts/queries.py Outdated Show resolved Hide resolved
accounts/tests.py Show resolved Hide resolved
studies/forms.py Show resolved Hide resolved
accounts/forms.py Show resolved Hide resolved
Lookit automation moved this from In progress to Review in progress Aug 8, 2019
Copy link
Contributor

@kimberscott kimberscott left a comment

Choose a reason for hiding this comment

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

Minor questions/comments only. This is great!

Much more strict.
@kimberscott
Copy link
Contributor

Lookit automation moved this from Review in progress to Reviewer approved Aug 8, 2019
@Datamance Datamance merged commit 046c82a into develop Aug 8, 2019
Lookit automation moved this from Reviewer approved to Done Aug 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Researcher [Audience] Researcher-facing
Projects
No open projects
Lookit
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants