Support for Full Text Search Query #541

dplewis · 2018-01-10T01:14:22Z

Supersed: #470
Closes: parse-community/parse-server#4490, #476

To be used with Parse-Server 2.5.0+

Fixed docs typos
Lint Test files
Remove unnecessary code from past tests
Bump parse-server to 2.7.1

codecov · 2018-01-10T01:36:15Z

Codecov Report

Merging #541 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #541      +/-   ##
==========================================
+ Coverage   84.59%   84.62%   +0.03%     
==========================================
  Files          48       48              
  Lines        3952     3960       +8     
  Branches      897      900       +3     
==========================================
+ Hits         3343     3351       +8     
  Misses        609      609

Impacted Files	Coverage Δ
src/ParsePolygon.js	`0% <ø> (ø)`	⬆️
src/ParseQuery.js	`94.13% <100%> (+0.12%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee33c48...3cd3f7c. Read the comment docs.

dplewis · 2018-01-10T01:48:33Z

@flovilmart and @SebC99 can you look over this?

SebC99 · 2018-01-10T08:20:58Z

Why don't you use the diacritics, language and casesensitive options?
As for the doc I know the rest call needs a key but mongo doesn't, so we shouldn't say in the docs that the key will be the search field as mongo will use the text index without looking at the key.
Apart from that I'm ok with it! Thanks!

dplewis · 2018-01-10T08:28:30Z

That is for Parse-Server as a service. We can include them but we have to think about the current and future database adapters. Postgres requires a key for example and you have to install an extension for diacritics option. We should go for something simple here out of the box. The RestApi should be sufficient for specific full text search queries.

montymxb

Alright, this looks pretty good for getting this in place. Some questions about removed code and suggestions for error test cases.

I wouldn't stress too much on the 'additional' features for full text, as this isn't going to be released just yet. It'll be nice to have this in place, we can guarantee we will at least have simplistic full text support in this sdk; rather than none.

montymxb · 2018-01-15T20:17:55Z

src/__tests__/ParseSchema-test.js

@@ -13,15 +13,6 @@ var ParseSchema = require('../ParseSchema').default;
 var ParsePromise = require('../ParsePromise').default;
 var CoreManager = require('../CoreManager');

-function generateSaveMock(prefix) {


Yeah I copied the jest test from another file when I added schema support. Forgot I left those in 😅

montymxb · 2018-01-15T20:18:10Z

src/__tests__/ParseSchema-test.js

      return ParsePromise.as([]);
    };
    var ajax = function(method, path, data, headers) {
-      var name = path.substr(path.indexOf('/') + 1);


Unused here as well?

montymxb · 2018-01-15T20:22:49Z

src/__tests__/ParseQuery-test.js

+    try {
+      const query = new ParseQuery('Item');
+      query.fullText();
+    } catch (e) {


Rather than just try/catching I would use expect(...).toThrow('my error...'), that would be much more explicit in terms of that we are expecting and what we are expecting to receive as the error. You can see an example in the parse-server tests.

montymxb · 2018-01-15T20:22:56Z

src/__tests__/ParseQuery-test.js

+    try {
+      const query = new ParseQuery('Item');
+      query.fullText('key');
+    } catch (e) {


montymxb · 2018-01-15T20:23:04Z

src/__tests__/ParseQuery-test.js

+    try {
+      const query = new ParseQuery('Item');
+      query.fullText('key', []);
+    } catch (e) {


dplewis · 2018-01-15T21:21:31Z

@montymxb Thanks for the review, changes added.

montymxb

Much better, I'd say this is approved .

dplewis · 2018-01-15T23:42:12Z

@montymxb I'll start working on the docs after this gets in

montymxb · 2018-01-15T23:43:26Z

That would be good, that and aggregate as mentioned in #543 (we do have that here right?).

dplewis · 2018-01-15T23:44:50Z

Yes, quick question should we have documentation in docs folder, the gh-pages branch or both?

flovilmart · 2018-01-15T23:46:21Z

The docs are automatically generated and push by Travis based on the JSdocs in the gh-pages branch, so no need to worry about it

montymxb · 2018-01-15T23:47:14Z

We'll want docs in the docs repo separately, but you should be good here.

dplewis · 2018-01-15T23:51:58Z

@flovilmart Shouldn't http://parseplatform.org/Parse-SDK-JS/api/master/ be in the README so new users can see it or am I missing something?

I can add some examples to be auto generated for aggregate like I did for this PR.

flovilmart · 2018-01-15T23:53:51Z

This is linked into http://docs.parseplatform.org/js/api

dplewis · 2018-01-15T23:59:51Z

@flovilmart Ah, I see. I'm so used to going to the guide in the readme instead of the api reference.

flovilmart · 2018-01-16T00:06:08Z

Yeah, the guide is for the usage, also we have the API references in each repositories

montymxb · 2018-01-16T00:27:06Z

We should have a link to the API reference on the README 🔖

flovilmart · 2018-01-16T00:47:44Z

yep those are:

http://parseplatform.org/Parse-SDK-JS/api -> lastest release

http://parseplatform.org/Parse-SDK-JS/api/v1.11.0/ -> tagged release

http://parseplatform.org/Parse-SDK-JS/api/master/ -> master branch

montymxb · 2018-01-16T00:59:03Z

@flovilmart probably should clarify specifically from the README on github repo page, not already from within the parseplatform.org location. https://github.com/parse-community/Parse-SDK-JS/.

flovilmart · 2018-01-16T01:00:04Z

Yes I agree ! let's do it, I just posted the links for reference.

srameshr · 2018-01-27T06:35:08Z

How do I install the version where this feature has been merged?

Im on Parse Server 2.7.1 which is installing Parse-JS-SDK @1.11.
The fullText is supported at master: http://parseplatform.org/Parse-SDK-JS/api/master/Parse.Query.html#fullText

Are you going to bump the version of JS-SDK and subsequently include the same version in Parse Server.

Its been 2 weeks since this has been merged. Hoping to see this merged to 1.11.0 at least without the need to upgrade parse server and parse server already points to 1.11.0.

@flovilmart @montymxb @dplewis

srameshr · 2018-01-29T07:01:09Z

Guys, can you release a version with this fix or update 1.11.0 to include this?

srameshr · 2018-01-30T12:56:02Z

Any updates?

montymxb · 2018-02-01T18:12:53Z

@srameshr since this is an enhancement we'll probably wait a bit to bundle this with other features (if any are pending) and any fixes we may want. There's no official release schedule here, but if you feel that what we currently have is sufficient for a release go ahead and open up a separate issue for that. We can focus on talking about a release there, rather than mixing topics on this PR thread.

srameshr · 2018-02-01T18:19:14Z

@montymxb Yup, will do that.
But, I also think opening up an issue just to remind some other fix has to be merged would not make much sense.

montymxb · 2018-02-01T19:28:36Z

If you open up another issue keep it focused on a request for a release and your reason why. We may still hold it back if we decide we want to add more than what we currently have. This isn't necessarily another fix that needs to be merged, this just brings it to our attention so we can evaluate it formally.

srameshr · 2018-02-01T19:33:56Z

Shouldn't what needs to be merged, be evaluated on current needs of the community / users instead of how big the fix is or you deciding what is more than what you currently have?

montymxb · 2018-02-01T19:42:58Z

Absolutely, so please open up an issue for that topic so we can all deliberate on that specifically. Since this is a topic for a given PR that has been merged I would consider what you're talking about as a separate topic, i.e. drafting a release. Moving the convo there will allow us to have that focused on input for said topic specifically, and if it makes sense we'll go ahead and draft.

dplewis added 2 commits January 9, 2018 19:11

Support for Full Text Search Query

7069564

test fix

7aa239d

dplewis mentioned this pull request Jan 11, 2018

Add the new fullTextSearch method with the associated tests. #470

Closed

montymxb suggested changes Jan 15, 2018

View reviewed changes

cleanup tests

3cd3f7c

montymxb approved these changes Jan 15, 2018

View reviewed changes

montymxb merged commit 7ef1b0a into parse-community:master Jan 15, 2018

dplewis deleted the full-text-search branch January 15, 2018 23:45

srameshr mentioned this pull request Feb 1, 2018

Support fullTextSearch in 1.11.0 #548

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for Full Text Search Query #541

Support for Full Text Search Query #541

dplewis commented Jan 10, 2018 •

edited

Loading

codecov bot commented Jan 10, 2018 •

edited

Loading

dplewis commented Jan 10, 2018 •

edited

Loading

SebC99 commented Jan 10, 2018

dplewis commented Jan 10, 2018 •

edited

Loading

montymxb left a comment

montymxb Jan 15, 2018

dplewis Jan 15, 2018 •

edited

Loading

montymxb Jan 15, 2018

montymxb Jan 15, 2018

montymxb Jan 15, 2018

montymxb Jan 15, 2018

dplewis commented Jan 15, 2018

montymxb left a comment

dplewis commented Jan 15, 2018

montymxb commented Jan 15, 2018

dplewis commented Jan 15, 2018

flovilmart commented Jan 15, 2018

montymxb commented Jan 15, 2018

dplewis commented Jan 15, 2018 •

edited

Loading

flovilmart commented Jan 15, 2018 •

edited by montymxb

Loading

dplewis commented Jan 15, 2018

flovilmart commented Jan 16, 2018

montymxb commented Jan 16, 2018

flovilmart commented Jan 16, 2018

montymxb commented Jan 16, 2018

flovilmart commented Jan 16, 2018

srameshr commented Jan 27, 2018 •

edited

Loading

srameshr commented Jan 29, 2018

srameshr commented Jan 30, 2018

montymxb commented Feb 1, 2018

srameshr commented Feb 1, 2018

montymxb commented Feb 1, 2018

srameshr commented Feb 1, 2018

montymxb commented Feb 1, 2018

Support for Full Text Search Query #541

Support for Full Text Search Query #541

Conversation

dplewis commented Jan 10, 2018 • edited Loading

codecov bot commented Jan 10, 2018 • edited Loading

Codecov Report

dplewis commented Jan 10, 2018 • edited Loading

SebC99 commented Jan 10, 2018

dplewis commented Jan 10, 2018 • edited Loading

montymxb left a comment

Choose a reason for hiding this comment

montymxb Jan 15, 2018

Choose a reason for hiding this comment

dplewis Jan 15, 2018 • edited Loading

Choose a reason for hiding this comment

montymxb Jan 15, 2018

Choose a reason for hiding this comment

montymxb Jan 15, 2018

Choose a reason for hiding this comment

montymxb Jan 15, 2018

Choose a reason for hiding this comment

montymxb Jan 15, 2018

Choose a reason for hiding this comment

dplewis commented Jan 15, 2018

montymxb left a comment

Choose a reason for hiding this comment

dplewis commented Jan 15, 2018

montymxb commented Jan 15, 2018

dplewis commented Jan 15, 2018

flovilmart commented Jan 15, 2018

montymxb commented Jan 15, 2018

dplewis commented Jan 15, 2018 • edited Loading

flovilmart commented Jan 15, 2018 • edited by montymxb Loading

dplewis commented Jan 15, 2018

flovilmart commented Jan 16, 2018

montymxb commented Jan 16, 2018

flovilmart commented Jan 16, 2018

montymxb commented Jan 16, 2018

flovilmart commented Jan 16, 2018

srameshr commented Jan 27, 2018 • edited Loading

srameshr commented Jan 29, 2018

srameshr commented Jan 30, 2018

montymxb commented Feb 1, 2018

srameshr commented Feb 1, 2018

montymxb commented Feb 1, 2018

srameshr commented Feb 1, 2018

montymxb commented Feb 1, 2018

dplewis commented Jan 10, 2018 •

edited

Loading

codecov bot commented Jan 10, 2018 •

edited

Loading

dplewis commented Jan 10, 2018 •

edited

Loading

dplewis commented Jan 10, 2018 •

edited

Loading

dplewis Jan 15, 2018 •

edited

Loading

dplewis commented Jan 15, 2018 •

edited

Loading

flovilmart commented Jan 15, 2018 •

edited by montymxb

Loading

srameshr commented Jan 27, 2018 •

edited

Loading