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

Using the encoder on state is too broad. #18

Merged
merged 1 commit into from
Mar 27, 2017

Conversation

rogerhu
Copy link
Contributor

@rogerhu rogerhu commented Mar 22, 2017

@mmimeault
Copy link
Contributor

Ok so as I understand you disable the encoding of the other states? Like the orderByAscending mentioned in the issue #14?

It make sense as the protocol only support className, where, but what about "fields": ["name"] // Optional ?

Thanks

@flovilmart
Copy link

We could also solve it server side, not reject those fields, and in the future, add support for those.

@rogerhu
Copy link
Contributor Author

rogerhu commented Mar 22, 2017

Yeah I don't see how fields is being used in the iOS version at this point. It feels like a separate issue.

Is it a matter of encoding selectKeys()?

Does fields also work for dotted notation?

@flovilmart
Copy link

@rogerhu fields doesn't filter server side IIRC

@rogerhu
Copy link
Contributor Author

rogerhu commented Mar 23, 2017

@mmimeault
Copy link
Contributor

Ok. mm so what is the conclusion, fine like that for now? Or we should include fields?

@rogerhu
Copy link
Contributor Author

rogerhu commented Mar 24, 2017

It sounds like we shouldn't include fields for now. We could add it as selectKeys manually but it doesn't even work on the Parse server. And the iOS version doesn't even include fields so we should decide whether we should add support in a different Pr.

@mmimeault
Copy link
Contributor

I think I'm fine with this PR for now. But we should "note" somewhere that fields is not supported by the server and neither by the SDK, so we know we have to do it later. (not forget)

@rogerhu
Copy link
Contributor Author

rogerhu commented Mar 27, 2017

Done: parse-community/parse-server#3671.
Added comment too

@rogerhu rogerhu merged commit 0f4d2fe into parse-community:master Mar 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants