-
Notifications
You must be signed in to change notification settings - Fork 27
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
Consider nested filters in the extra_kwargs #48
Conversation
This is great! Thanks! Can I ask you to update the |
Should be ready. Let me know if you run into any hiccups. Thanks for considering it! |
@pablodiazgutierrez Looks like tests are failing. Can you take a look? |
Good catch. I think the PR in #52 should solve it. But I'm not sure how to run the tests locally. Do you have a tip to point a newbie in the right direction? |
👍 Yeah, is usually use tox from the project root and that runs the tests for all versions of Python, this requires multiple versions of python installed though. If you don't have multiple versions available, |
Ok neat, thanks. Using tox I see that tests for the PR above run fine on Python 3.8, so hopefully that’s a good sign.
…-Pablo
From: Paul Logston <notifications@github.com>
Reply-To: 15five/django-scim2 <reply@reply.github.com>
Date: Tuesday, October 27, 2020 at 6:56 PM
To: 15five/django-scim2 <django-scim2@noreply.github.com>
Cc: Pablo Diaz-Gutierrez <pablo@appfluence.com>, Mention <mention@noreply.github.com>
Subject: Re: [15five/django-scim2] Consider nested filters in the extra_kwargs (#48)
👍 Yeah, is usually use tox from the project root and that runs the tests for all versions of Python, this requires multiple versions of python installed though. If you don't have multiple versions available, python setup.py test will do the trick.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#48 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AA5GMCAAWCKOGGHQJYA7NRDSM525XANCNFSM4SVMVS6Q>.
|
This has been released as 0.16.4. |
The get_nested_field() function is a really nice addition to this library, but doesn't seem to work for reverse relationships. I have the same use case as Pablo (needing to filter by company in a multitennant SAAS). However in my use case the object relationship direction of the object is a reverse one, (Example Model below). I'm using this line in get_extra_filter_kwargs:
Similar to Pablo's original issue, for requests to /Users?filter=userName+eq+"example_username" the result is blank, but /Users/1 and Users?attributes=userName do work correctly, and apply the company restrictions as expected. I think I've diagnosed that the hasattr() line (shown below) that was added in this merge is causing this the filtering to stop prematurely.
Example Model
|
Hi @William-Wildridge, those kwargs are used here django-scim2/src/django_scim/views.py Lines 88 to 94 in b3abe0f
/User/1 )
and here django-scim2/src/django_scim/views.py Lines 178 to 179 in b3abe0f
/Users?filter=userName+eq+"example_username" ).
The search logic leads here django-scim2/src/django_scim/views.py Line 204 in b3abe0f
django-scim2/src/django_scim/views.py Lines 189 to 190 in b3abe0f
Since your user object ( Unfortunately I don't have time to update this code to handle the reverse relationships. But you could try to filter with a different set of args and add a method to your user class that would return the company manually. Eg. def get_extra_filter_kwargs(request, *args, **kwargs):
return {
'company': request.user.memberships.first().company,
} class User:
@property
def company(self):
return self.memberships.first().company It's hacky IMO, but it could work. |
Allow nested filters to be applied to the search results. So if we get
{"account__admin__id": 123}, we compare the appropriate nested field. Right now, it understands
"account__admin__id"` to be a field name, which is incorrect.Discussed in issue #47 and tested successfully in my local setup.