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

Adds support for like/notLike and contains/notContains operators #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

recipher
Copy link

Hi. After further investigation, I've added support for contains/notContains matching against an array field, and like/notLike matching against a string field. The like/notLike operators bring parity with the js-data-sql adapter.

Unfortunately, I don't believe it's possible to use a single operator to handle both cases - against a string field or an array field. Using two operators is necessary, since there's no way from the adapter to determine which type the field has.

Thanks.

@jmdobry
Copy link
Member

jmdobry commented Sep 22, 2015

That's probably a good approach, one that we should adopt across js-data and all of the adapters.

@recipher
Copy link
Author

Any decision on this?

@jmdobry
Copy link
Member

jmdobry commented Nov 13, 2015

I'll take a look tonight, but for starters:

This branch has conflicts that must be resolved

@recipher
Copy link
Author

Thanks. findAll specs use yield now. I'll fix them up.

@recipher
Copy link
Author

I should point out, I didn't commit the distribution (against the contributing rules), it's been updated in this commit after I merged from master. Cheers.

@jmdobry
Copy link
Member

jmdobry commented Nov 13, 2015

ty

@jmdobry
Copy link
Member

jmdobry commented Mar 18, 2016

If you want to get this up-to-date with master and squash into 1 commit, then I'll merge and rebase v3 so v3 can get support for like/notLike (v3 already has support for contains/notContains).

@jmdobry
Copy link
Member

jmdobry commented Mar 18, 2016

Also, remove from this PR your changes to the dist/ file, thanks!

@calmdev
Copy link

calmdev commented Jul 2, 2016

Will these operators be made available in a version 2 release? I'm trying them out now. Using the http adapter to interact with a rethinkdb backend that's filtering data sets on the front-end.

@jmdobry
Copy link
Member

jmdobry commented Jul 2, 2016

Well, right now they're not available. I can't merge this PR until the conflicts are fixed.

@calmdev
Copy link

calmdev commented Jul 2, 2016

Right, I'd like to help if I can, but it's unclear to me if this will only be merged into version 3, or if this will also land in version 2. I'm hoping version 2 can receive a feature release. Does this require a PR against both v2 and master branch? It's been 9 months since the PR was opened. What exactly is still applicable from this PR? Only the additions made to src/index.js? I'm not seeing how the spec test for the findAll method applies to the branches. Maybe things have just changed since the PR was originally made?

@calmdev
Copy link

calmdev commented Jul 2, 2016

I am noticing that when where is present it doesn't seem to acknowledge offset or limit.

@jmdobry
Copy link
Member

jmdobry commented Jul 2, 2016

Yeah, things have changed quite a bit. Perhaps I should just extract the parts that are still relevant.

@calmdev
Copy link

calmdev commented Jul 9, 2016

If this makes it into version 2 and you need a tester let me know. For now, I'm working around my issue using the underlying rethinkdbdash instance. Looking forward to version 3 of js-data 🚀

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

Successfully merging this pull request may close these issues.

3 participants