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

Allow user search with multiple attributes #21

Open
tkharju opened this Issue Jan 18, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@tkharju
Contributor

tkharju commented Jan 18, 2016

Currently query /api/1/user?dreamschool=123&foo=bar filters UserAttributes by only picking either of the parameters (randomly).

DoD: User queryset is filtered using all GET parameters

Ref:

break # only handle one GET variable for now

This is the reason why following test failed: https://travis-ci.org/educloudalliance/eca-auth-data/jobs/102996338

tkharju added a commit to haltu/eca-auth-data that referenced this issue Jan 18, 2016

@tkharju tkharju referenced this issue Jan 18, 2016

Merged

Fix failing test #22

@derega

This comment has been minimized.

Show comment
Hide comment
@derega

derega Jan 18, 2016

Member

The documentation for that endpoint says:

Query is made by GET parameters. Only one parameter is allowed. The parameter
consists of an attribute name and an attribute value.

Where and why this endpoint should support more than one query parameter?

Member

derega commented Jan 18, 2016

The documentation for that endpoint says:

Query is made by GET parameters. Only one parameter is allowed. The parameter
consists of an attribute name and an attribute value.

Where and why this endpoint should support more than one query parameter?

@derega

This comment has been minimized.

Show comment
Hide comment
@derega

derega Jan 18, 2016

Member

The failing test:

def test_get_object_with_attributes(self, requests_mock):

Member

derega commented Jan 18, 2016

The failing test:

def test_get_object_with_attributes(self, requests_mock):

@tkharju

This comment has been minimized.

Show comment
Hide comment
@tkharju

tkharju Jan 18, 2016

Contributor

We discussed about this with @derega and came to the conclusion that it would be better to do the tests at this point only with one attribute. In other words mpass-data works so that /api/1/user?dreamschool=123 is valid but /api/1/user?dreamschool=123&foo=bar is not.
Current implementation is not deterministic. If user makes query against the spec by giving two or more attributes, it is possible that at times the results differ, as only the first attribute is used for filtering but the order of get-attributes may change even between similar requests.

Contributor

tkharju commented Jan 18, 2016

We discussed about this with @derega and came to the conclusion that it would be better to do the tests at this point only with one attribute. In other words mpass-data works so that /api/1/user?dreamschool=123 is valid but /api/1/user?dreamschool=123&foo=bar is not.
Current implementation is not deterministic. If user makes query against the spec by giving two or more attributes, it is possible that at times the results differ, as only the first attribute is used for filtering but the order of get-attributes may change even between similar requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment