Skip to content

Add ability to search Object columns #727

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

Merged
merged 4 commits into from
Jun 14, 2017
Merged

Conversation

ssamuli
Copy link
Contributor

@ssamuli ssamuli commented Jun 13, 2017

As requested in issue #720

What do you think, is this something you were looking for?

screen shot 2017-06-13 at 20 19 45

screen shot 2017-06-13 at 20 19 17

@natanrolnik
Copy link
Contributor

natanrolnik commented Jun 13, 2017

OMG this is fantastic!!!
This is good, but considering we are dealing with this, do you think we could have does not contain, greater than/greater than or equal to, less than/less than or equal to? That would be perfect!!
If not, we can merge and improve it later.

@oferRounds
Copy link

@ssamuli - THANK YOU, just saw it now, looks awesome! That would really be useful!

exists, does not exist, equal to, not equal to, greater than, greater than or equal, less than, less than or equal.
@ssamuli
Copy link
Contributor Author

ssamuli commented Jun 13, 2017

@natanrolnik good idea. OK, now this should cover all.

@ssamuli
Copy link
Contributor Author

ssamuli commented Jun 13, 2017

@oferRounds happy to be of help!

function addQueryConstraintFromObject(query, filter, constraintType) {
let compareTo = JSON.parse(filter.get('compareTo'));
for (let key of Object.keys(compareTo)) {
query[constraintType](filter.get('field')+'.'+key, compareTo[key]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, I didn't know about the query['equalTo'](..., ...) syntax...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an old javascript feature, I'm just accessing the function by its name through bracket notation https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Property_Accessors#Bracket_notation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to know! Thank you.

@natanrolnik
Copy link
Contributor

natanrolnik commented Jun 14, 2017

@ssamuli thanks once again for your contribution, this is amazing. I'm sorry to be picky here, but I think it's important to get it right before merging - I have two notes:

1 - the key exists and key does not exist are not working for me, or at least I don't understand how I should write the key. Not only that, for these options, it doesn't save the state when I open the Filter popup again.

I guess the problem is in the following code:

let compareTo = JSON.parse(filter.get('compareTo'));
     for (let key of Object.keys(compareTo)) {

And I guess you need a function different than:
function addQueryConstraintFromObject(query, filter, constraintType) {

2 - Do you think you could add a exists and does not exist option also, being the first ones, for the object itself, and not for the keys? I mean, if I want to search for Parse Object where a object column exists or not, in this current implementation it's not possible.

@ssamuli
Copy link
Contributor Author

ssamuli commented Jun 14, 2017

  1. Key exists and does not exist works with {"key.im.looking.for":1} for example, did you try that? Should work, at least it did last night when I tried it.

  2. Sure, I'll add those too.

@natanrolnik
Copy link
Contributor

natanrolnik commented Jun 14, 2017

I didn't try this way, but I would expect it to work just with a plain string. The example you gave requires understanding how the underlying code works (now I understand why it works). That's why I suggested a different function to modify the query that could handle only a string and not a JSON object for key exists and key does not exist.

@ssamuli
Copy link
Contributor Author

ssamuli commented Jun 14, 2017

Yes it's a bit tricky to use but it's kind of consistent with how the rest of the object search works, that's why I thought it was acceptable. I'll change that to work with strings as you suggested.

@ssamuli
Copy link
Contributor Author

ssamuli commented Jun 14, 2017

There you are 🙂

@ssamuli
Copy link
Contributor Author

ssamuli commented Jun 14, 2017

Btw. would be great feature if those search options had placeholders showing the user how the input should look like. Not just for this but for everything.

@natanrolnik
Copy link
Contributor

Tested now and it's perfect. Let's 🚢 it!
Thanks @ssamuli!

@natanrolnik natanrolnik merged commit 325afa6 into parse-community:master Jun 14, 2017
@ssamuli
Copy link
Contributor Author

ssamuli commented Jun 14, 2017

Thank you! 🎉

@ssamuli ssamuli deleted the dev branch June 14, 2017 10:12
@natanrolnik
Copy link
Contributor

Closes #720

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