-
Notifications
You must be signed in to change notification settings - Fork 13
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
Fix person and organization filter for affiliations #96
Conversation
So, a |
@emiliom How do I go about that? Sorry.. not SQLAlchemy pro here 😉 |
Neither am I! But I'd look for similar examples in other query functions. If it is in fact doable, it seems cleaner to reuse that "predefined" relationship rather than adding explicit joins. |
odm2api/ODM2/services/readService.py
Outdated
q = q.join(People) | ||
if personfirst: q = q.filter(People.PersonFirstName.ilike(personfirst)) | ||
if personlast: q = q.filter(People.PersonLastName.ilike(personlast)) | ||
if orgcode: q = q.join(Affiliations.OrganizationObj).filter( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@emiliom Is this what you're looking for? I still have to do the join... Can't find anything that simply filters straight up from relationship.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. That doesn't look better 😞 After looking at what's used in If you've tested this, let's go with it. It would seem cleaner and possibly more efficient (in some cases) to do all the joining in the Thanks. |
Let me know if you've tested this commit, and I'll accept the PR. |
This has been tested and good to go with your approval. Thanks! |
Overview
This PR addresses #95. It joins the necessary table to
Affiliations
table so that filter can actually occur. I have tested it and it seems to work.