Add options to full text search #573

danielbm · 2018-05-22T02:30:49Z

This PR adds the support for options on the full text search, namely: $caseSensitive, $language and $diacriticSensitive.

codecov · 2018-05-22T12:13:37Z

Codecov Report

Merging #573 into master will increase coverage by <.01%.
The diff coverage is 89.65%.

@@            Coverage Diff             @@
##           master     #573      +/-   ##
==========================================
+ Coverage   84.57%   84.57%   +<.01%     
==========================================
  Files          48       48              
  Lines        4019     4039      +20     
  Branches      906      911       +5     
==========================================
+ Hits         3399     3416      +17     
- Misses        620      623       +3

Impacted Files	Coverage Δ
src/ParseQuery.js	`93.8% <89.65%> (-0.43%)`	⬇️

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 acdb645...249edb1. Read the comment docs.

dplewis · 2018-05-22T16:42:41Z

@flovilmart Can you take a look at travis?

flovilmart · 2018-05-22T17:10:01Z

@dplewis seems that integration tests are failing. Not sure why or if we broke something else.

dplewis · 2018-05-22T18:52:04Z

@flovilmart Thanks for fixing the tests

flovilmart · 2018-05-22T19:04:50Z

NP!

dplewis · 2018-05-22T19:08:36Z

src/ParseQuery.js

  * @return {Parse.Query} Returns the query, so you can chain this call.
  */
- fullText(key: string, value: string): ParseQuery {
+ fullText(key: string, value: string, options: ?Object): ParseQuery {


Can you add something similar to other functions that use options?

https://github.com/danielbm/Parse-SDK-JS/blob/4ea0c11bd375f0f1c8f942b7308e57ad9ba6ce30/src/ParseQuery.js#L495

I mean something like this for every option.

var fullOptions = {}; if (options.hasOwnProperty('language')) { fullOptions.$language = options.language; }

Can you also write a test case for $caseSensitive, $diacriticSensitive, and options.unknown (this might throw an error)?

Do you think this code would do the trick?

options = options || {}; var fullOptions = {}; for (var key in options) { switch (key) { case '$language': fullOptions.$language = options[key]; break; case '$caseSensitive': fullOptions.$caseSensitive = options[key]; break; case '$diacriticSensitive': fullOptions.$diacriticSensitive = options[key]; break; default: throw new Error('Unknown option: ' + key); break; } }

It should do the trick. Also I don't think we should have the $ in the options getting passed in. $ is an internal thing. What do you think?

@danielbm Can you write the test cases first?

It seemed a bit verbose to me too, but doing the validation for the three options separately, plus the validation for unknown options was also. As per the $, I totally agree. I'll remove it.

@dplewis I just pushed the code with the options validations, more test cases.

dplewis · 2018-05-23T13:09:20Z

@flovilmart is there an easy way to view generated docs for a PR without pulling it down?

flovilmart · 2018-05-23T13:13:02Z

Not really ;/ you’ll have to pull it

dplewis · 2018-05-23T13:28:42Z

@flovilmart I think the CONTRIBUTION.md looks outdated. Doesn’t say how to run those unit jest test, integration test, generate docs, etc.

flovilmart · 2018-05-23T13:30:23Z

@dplewis you're right ;) we need to do somethings about that

dplewis

A few nits

dplewis · 2018-05-23T18:02:26Z

src/ParseQuery.js

+      }
+    }
+
+    if (options && options.hasOwnProperty('$diacriticSensitive')) {


Remove this

dplewis · 2018-05-23T18:02:31Z

src/ParseQuery.js

@@ -993,7 +993,7 @@ class ParseQuery {
  *
  * In order to sort you must use select and ascending ($score is required)
  *  <pre>
-  *   query.fullText('term');
+  *   query.fullText('field', 'term', { $language: "spanish" });


For documentation purposes leave an example of with and w/o options.

Docs are auto generated by using npm run docs. Use it to check your formatting.

$language is now language

dplewis · 2018-05-23T18:06:23Z

src/ParseQuery.js

+  * @param {Object} options (Optional) See https://docs.mongodb.com/manual/reference/operator/query/text/#text-query-operator-behavior
+  * @param {String} options.language
+  * @param {String} options.caseSensitive
+  * @param {String} options.diacriticSensitive


Diacritic and case are Boolean type. What happens if they aren't boolean 🤔?

Instead of the mongo docs, put a summary next to the options for readability

Like this?

/** * Adds a constraint for finding string values that contain a provided * string. This may be slow for large datasets. Requires Parse-Server > 2.5.0 * * In order to sort you must use select and ascending ($score is required) * <pre> * query.fullText('field', 'term'); * query.ascending('$score'); * query.select('$score'); * </pre> * * To retrieve the weight / rank * <pre> * object->get('score'); * </pre> * * You can define optionals by providing an object as a third parameter * <pre> * query.fullText('field', 'term', { language: "spanish", diacriticSensitive: true }); * </pre> * * @param {String} key The key that the string to match is stored in. * @param {String} value The string to search * @param {Object} options (Optional) See https://docs.mongodb.com/manual/reference/operator/query/text/#text * @param {String} options.language The language that determines the list of stop words for the search and the rules for the stemmer and tokenizer. * @param {Boolean} options.caseSensitive A boolean flag to enable or disable case sensitive search. * @param {Boolean} options.diacriticSensitive A boolean flag to enable or disable diacritic sensitive search. * @return {Parse.Query} Returns the query, so you can chain this call. */

dplewis · 2018-05-23T18:06:50Z

src/ParseQuery.js

-
-   return this._addCondition(key, '$text', { $search: { $term: value } });
- }
+  fullText(key: string, value: string, options: ?Object): ParseQuery {


Looks like this is indented improperly

Actually it was indented improperly before, and I fixed it.

dplewis · 2018-05-23T18:10:10Z

integration/test/ParseQueryTest.js

+    });
+  });
+
+  it('can perform a full text search with case sensistive options', (done) => {


*case sensitive

dplewis · 2018-05-24T15:01:43Z

integration/test/ParseQueryTest.js

+      q.fullText('comment', 'preparar', { language: "portuguese" });
+      return q.find();
+    }).then((results) => {
+      assert.equal(results.length, 4);


Can you write this like? It kinda confusing in the tests.

}).then((results) => { assert.equal(results.length, 4); done(); });

dplewis · 2018-05-24T15:03:12Z

integration/test/ParseQueryTest.js

+    }
+    Parse.Object.saveAll(objects).then(() => {
+      const q = new Parse.Query(TestObject);
+      q.fullText('comment', 'Preparar', { language: "portuguese", caseSensistive: true });


I don't know how this test is passing when caseSensistive is spelled wrong.

dplewis · 2018-05-24T15:04:03Z

@danielbm I added a few more comments on your test cases

cleever · 2018-06-26T22:11:10Z

Any news on this?

danielbm · 2018-06-26T22:20:24Z

@cleever Not really. I'm just having difficulties to find time to fix this PR.

dplewis · 2018-06-26T22:21:32Z

@danielbm I can finish off this PR for you, if thats cool.

danielbm · 2018-06-26T22:22:58Z

@dplewis Sure! Feel free to do it!

Indexes don't get deleted between tests so new classes are created as a workaround. Also I can't seem to use language options with case sensitive or diacritic sensitive

flovilmart · 2018-07-04T15:58:16Z

@dplewis where are we at on this one?

dplewis · 2018-07-04T16:00:06Z

@danielbm how does this look?

flovilmart

LGTM too

danielbm · 2018-07-04T17:44:52Z

@dplewis @flovilmart thanks!

flovilmart · 2018-07-04T17:50:37Z

Thanks @danielbm for getting it started!

danielbm added 4 commits March 7, 2018 00:07

Adding options to fullText search

4266952

Merge remote-tracking branch 'oficial/master'

0808cf1

adding tests

57eb5cd

fix tests

a2b1b5b

Merge branch 'master' into master

4ea0c11

dplewis suggested changes May 22, 2018

View reviewed changes

danielbm added 3 commits May 22, 2018 16:52

add options validation

9eacfcd

Merge branch 'master' of https://github.com/danielbm/Parse-SDK-JS

d3a3158

add options validation and more test cases

93b7677

removing commented out code

a0db1d3

dplewis suggested changes May 23, 2018

View reviewed changes

dplewis reviewed May 24, 2018

View reviewed changes

dplewis added 3 commits June 26, 2018 17:24

Merge branch 'master' into master

f2056b1

add more test

b5c0063

sort by text score method

9b4ee93

dplewis added 2 commits June 30, 2018 17:32

Text fix

01c8170

Indexes don't get deleted between tests so new classes are created as a workaround. Also I can't seem to use language options with case sensitive or diacritic sensitive

Merge branch 'master' into master

249edb1

dplewis closed this Jul 4, 2018

dplewis reopened this Jul 4, 2018

dplewis approved these changes Jul 4, 2018

View reviewed changes

flovilmart approved these changes Jul 4, 2018

View reviewed changes

dplewis merged commit de4bc1c into parse-community:master Jul 4, 2018

Uh oh!

Add options to full text search #573

Add options to full text search #573

Uh oh!

Conversation

danielbm commented May 22, 2018

Uh oh!

codecov bot commented May 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dplewis commented May 22, 2018

Uh oh!

flovilmart commented May 22, 2018

Uh oh!

dplewis commented May 22, 2018

Uh oh!

flovilmart commented May 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dplewis May 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dplewis May 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dplewis commented May 23, 2018

Uh oh!

flovilmart commented May 23, 2018

Uh oh!

dplewis commented May 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flovilmart commented May 23, 2018

Uh oh!

dplewis left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dplewis May 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dplewis May 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented May 22, 2018 •

edited

Loading

dplewis May 22, 2018 •

edited

Loading

dplewis May 23, 2018 •

edited

Loading

dplewis commented May 23, 2018 •

edited

Loading

dplewis left a comment •

edited

Loading

dplewis May 23, 2018 •

edited

Loading

dplewis May 24, 2018 •

edited

Loading

danielbm commented Jun 26, 2018 •

edited

Loading