-
Notifications
You must be signed in to change notification settings - Fork 4
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
Implement Order Filter for Python ICAT Backend #158
Implement Order Filter for Python ICAT Backend #158
Conversation
- This will remove circular imports in a later change - Also helps to tidy up the code base by separating the filter handler from the abstract classes of the filters
- If there's more than one order filter in a request, the others get overwritten due to the way setOrder() works in Python ICAT
- This is to remove a circular dependency that I found while implementing an order filter - Also added a couple of __init__.py files which didn't previously exist
common/icat/helpers.py
Outdated
@@ -394,3 +387,19 @@ def get_entity_with_filters(client, table_name, filters): | |||
raise MissingRecordError("No results found") | |||
else: | |||
return data | |||
|
|||
|
|||
def manage_order_filters(filters): |
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.
This could probably be given a more descriptive name - like clear_order_filters
?
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.
I've just pushed a change which renames this function and where it's used, definitely prefer the new name.
This PR will close #140.
This PR adds the functionality of order filters in get by filter requests. Multiple order filters can be added to a request, and they'll be added in the order which they're given in the request.
Due to a circular dependency, I've moved the location of
create_condition()
fromcommon/filters.py
tocommon/icat/filters.py
which seems sensible considering that's an Python ICAT specific piece of functionality.All unit tests that passed before this change, still pass. The 4 failures are related to
/sessions
endpoints and will be fixed when I adapt the unit tests to run on the Python ICAT backend.Once #153 has been merged into
master
, I will change the base branch of this PR to bemaster
(it's currentlypython-icat-where-filter-#142
as that's what I branched from originally).