Search issues fix by claudiaHash · Pull Request #641 · oceanprotocol/market

claudiaHash · 2021-06-03T10:05:22Z

Fixes #619 .

Changes proposed in this PR:

search by datatoken address, owner address, asset id added to existent elasticsearch query

vercel · 2021-06-03T10:05:24Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/oceanprotocol/market/6AZJVnxziczyxZEjD2cb8xLScgk9
✅ Preview: https://market-git-fix-619-search-issues-oceanprotocol.vercel.app

mihaisc

Looks good, not sure if you want to add more since it's still in draft.
Fixes just a small issue from #619.

claudiaHash · 2021-06-03T11:32:04Z

Looks good, not sure if you want to add more since it's still in draft.
Fixes just a small issue from #619.

There are still some issues unresolved yet that I'm working on.

mihaisc · 2021-06-07T14:05:24Z

+              should: [
+                {
+                  query_string: {
+                    query: `${searchTerm}`,


searchTerm differs in query_string you need to manually add OR between the words . in match_prase it's exactly the string the user inputs.

mihaisc · 2021-06-07T14:05:43Z

+                      'service.attributes.additionalInformation.description',
+                      'service.attributes.additionalInformation.tags'
+                    ],
+                    default_operator: 'AND',


remove this line, it shouldn't be AND

I added 'AND' as default op because using OR would give a match for every word in search string. For "worldlist used by" I got matches for assets that only contain "used" in their description/ title/ etc

yes, please use the searchTerm formated as described above with 'OR' between each word. "worldlist used by" would become "worldlist OR used OR by", just in query_string, not in match_phrase. Because you didn't add OR the query wasn't working properly that's why the results were not ok

Didn't used 2 different search terms on query_string and match_phrase there's where the problem came from. Please ignore the comments.

mihaisc · 2021-06-07T14:06:43Z

      `(service.attributes.additionalInformation.categories:\"${categories}\")`
    : text || ''

+  if (searchTerm.startsWith('did:op:')) {


why do we need this?

Removed it because searching by id that included "did:op:" in the beginning gave no result. Since the address that follows differentiates the assets, I thought that could be a solution.

it should work without removing did:op. Comment it for now. It work ok in my tests without removing did:op

searching for a2B8b3aC4207CFCCbDe4Ac7fa40214fd00A2BA71 is not working anyway

mihaisc · 2021-06-07T14:07:06Z

+    searchTerm = searchTerm.substring(7)
+  }
  // HACK: resolves the case sensitivity related to dataTokenInfo.symbol
  searchTerm = '*' + searchTerm.toUpperCase() + '*'


stop adding * at the start and end of the string

Added them for the cases user doesn't type all the search word. When searching by token symbol I found the following problem:

but it creates problems in other places. Searching for datatoken symbol will be fixed in oceanprotocol/aquarius#492

Oh, ok then I'll remove it. That was the only issue * resolved.

mihaisc · 2021-06-07T15:06:20Z

We'll tweak the query after these changes are implemented.

claudiaHash · 2021-06-07T16:24:19Z

I added that did:op: hack because I found that if the query_string and match_phrase search terms don't match case wise (only in that particular case) we get a server error:

It worked after that:

mihaisc · 2021-06-08T09:46:51Z

Ok, i'll play with it a bit and see how relevant are the result and if i can improve the query even more.

claudiaHash · 2021-06-08T10:22:48Z

Not sure if a - separated search input should give results for the whole string or if it's ok if terms are separated. Eg. mayors-office gives results that match: "office", "mayors-office"

mihaisc · 2021-06-08T11:10:04Z

it's ok, the secret is that we order them properly. We need to ad the sort criteria, after score. Currently they are sorted by created or if no sort filter is specified aquarius sorts them by id (it's not ok, an issue was created). The scope is to create multiple queries and order the matches. For example in the current queries match_phrase is the most strict and will be first in the results.

Signed-off-by: mihaisc <mihai.scarlat@smartcontrol.ro>

mihaisc · 2021-06-09T08:29:38Z

I removed the SearchQuery return type because we can't use it and i'm not sure why even bother. The structure of the es query is too complex and i see no value in trying to map in typescript

Signed-off-by: mihaisc <mihai.scarlat@smartcontrol.ro>

mihaisc · 2021-06-14T14:36:41Z

It would be good to include results where there is a partial match. For example, if you search "data union" you'll find that "dataunion.app" doesn't appear in the results

While this seems to be possible (check links below) it is far too complex to handle in this pr. It is nice to have indeed and I will create a new issue for it

https://www.elastic.co/guide/en/elasticsearch/reference/current/analysis-word-delimiter-tokenfilter.html
https://www.javacodegeeks.com/2018/03/elasticsearch-ignore-special-characters-query-pattern-replace-filter-custom-analyzer.html
https://www.elastic.co/guide/en/elasticsearch/reference/current/analysis-shingle-tokenfilter.html

Signed-off-by: mihaisc <mihai.scarlat@smartcontrol.ro>

…col/market into fix/619-search-issues

Signed-off-by: mihaisc <mihai.scarlat@smartcontrol.ro>

mihaisc · 2021-06-14T15:01:22Z

There already a lot of improvements here. I have commented Relevance sort until it's fixed. There are some other issues in aqua but i don't want to block this pr based on them. It's already good enough. Created 2 other issues based on things found here.

One last test from someone else please.

kremalicious · 2021-06-14T17:46:32Z

using this new search strategy in the background is already super valuable and seems to work well after some quick tests. One important, because very prominent, search which I found to be not working at all is the owner search, so the prominent publisher link on asset details page. When clicking that it looks to me as there is no search filtering happening at all, only lists recent items, e.g. https://market-git-fix-619-search-issues-oceanprotocol.vercel.app/search/?owner=0x655eFe6Eb2021b8CEfE22794d90293aeC37bb325&sort=created&sortOrder=desc

mihaisc · 2021-06-14T21:03:58Z

Ok, thanks on that. I remember now, i wanted to remove those but didn't finish. This way there is a bug if you click on your link and then search for something else (delete address and type something else ) nothing will happen.

old, bugs found

Signed-off-by: mihaisc <mihai.scarlat@smartcontrol.ro>

qlty-cloud-legacy · 2021-06-15T09:57:26Z

Code Climate has analyzed commit 44bb3aa and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Duplication	2

View more on Code Climate.

mihaisc · 2021-06-15T09:59:38Z

Ok, it should be working now ok.

claudiaHash · 2021-06-15T10:13:39Z

Ok, it should be working now ok.

The search after link clicking works perfectly. Since what I tested, the rest of the search works as it did before the last push. I think we could finally merge it.

jamiehewitt15 · 2021-06-15T13:06:28Z

It's a big improvement on before, all of the issues I mentioned above are fixed now.

claudia.holhos added 4 commits June 2, 2021 15:13

search by addresses

a540011

search by asset id

997fc93

logs deleted

3e9e3d4

Merge branch 'main' into fix/619-search-issues

e36599f

mihaisc self-requested a review June 3, 2021 11:30

mihaisc previously approved these changes Jun 3, 2021

View reviewed changes

search query updated

5b24ff0

vercel Bot deployed to Preview June 3, 2021 19:37 View deployment

search query updated

7bcec78

vercel Bot deployed to Preview June 7, 2021 13:03 View deployment

claudiaHash changed the title ~~Fix/619 search issues~~ Search issues fix Jun 7, 2021

mihaisc reviewed Jun 7, 2021

View reviewed changes

claudia.holhos added 2 commits June 7, 2021 19:07

search terms differentiated

5f8cc8b

asset id hack

d2900d7

vercel Bot deployed to Preview June 7, 2021 16:18 View deployment

id search hack removed

3647046

vercel Bot deployed to Preview June 8, 2021 08:32 View deployment

mihaisc added 2 commits June 9, 2021 11:25

restore lock file

c172cff

Signed-off-by: mihaisc <mihai.scarlat@smartcontrol.ro>

remove SearchQuery return type

b941615

Signed-off-by: mihaisc <mihai.scarlat@smartcontrol.ro>

vercel Bot deployed to Preview June 9, 2021 08:28 View deployment

refine query, add relevance sort

2468b77

Signed-off-by: mihaisc <mihai.scarlat@smartcontrol.ro>

vercel Bot requested a deployment to Preview June 9, 2021 09:16 Pending

vercel Bot deployed to Preview June 10, 2021 13:26 View deployment

mihaisc added 3 commits June 14, 2021 17:41

Merge branch 'main' into fix/619-search-issues

0d3430f

comment sort by relevance

f9d77ae

Signed-off-by: mihaisc <mihai.scarlat@smartcontrol.ro>

Merge branch 'fix/619-search-issues' of https://github.com/oceanproto…

6ec7912

…col/market into fix/619-search-issues

vercel Bot deployed to Preview June 14, 2021 14:47 View deployment

lint

4aa3dd5

Signed-off-by: mihaisc <mihai.scarlat@smartcontrol.ro>

vercel Bot deployed to Preview June 14, 2021 14:49 View deployment

mihaisc marked this pull request as ready for review June 14, 2021 14:59

mihaisc requested a review from kremalicious as a code owner June 14, 2021 14:59

search by owner and tags fix

68198d3

vercel Bot deployed to Preview June 14, 2021 18:39 View deployment

lint err fix

ca72b8a

vercel Bot deployed to Preview June 14, 2021 19:36 View deployment

mihaisc added the Status: DoNotMerge label Jun 14, 2021

mihaisc removed the Status: DoNotMerge label Jun 14, 2021

fix search

44bb3aa

Signed-off-by: mihaisc <mihai.scarlat@smartcontrol.ro>

vercel Bot deployed to Preview June 15, 2021 09:57 View deployment

jamiehewitt15 approved these changes Jun 15, 2021

View reviewed changes

claudiaHash merged commit eb8c6af into main Jun 15, 2021

claudiaHash deleted the fix/619-search-issues branch June 15, 2021 13:46

jamiehewitt15 mentioned this pull request Jun 30, 2021

Global search bar #690

Merged

Uh oh!

Conversation

claudiaHash commented Jun 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel Bot commented Jun 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mihaisc 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!

claudiaHash commented Jun 3, 2021

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!

mihaisc Jun 7, 2021 • 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!

claudiaHash Jun 7, 2021 • 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!

mihaisc commented Jun 7, 2021

Uh oh!

claudiaHash commented Jun 7, 2021

Uh oh!

mihaisc commented Jun 8, 2021

Uh oh!

claudiaHash commented Jun 8, 2021

Uh oh!

mihaisc commented Jun 8, 2021

Uh oh!

mihaisc commented Jun 9, 2021

Uh oh!

mihaisc commented Jun 14, 2021

Uh oh!

mihaisc commented Jun 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kremalicious commented Jun 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mihaisc commented Jun 14, 2021

Uh oh!

qlty-cloud-legacy Bot commented Jun 15, 2021

Uh oh!

mihaisc commented Jun 15, 2021

Uh oh!

claudiaHash commented Jun 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jamiehewitt15 commented Jun 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

claudiaHash commented Jun 3, 2021 •

edited

Loading

vercel Bot commented Jun 3, 2021 •

edited

Loading

mihaisc left a comment •

edited

Loading

mihaisc Jun 7, 2021 •

edited

Loading

claudiaHash Jun 7, 2021 •

edited

Loading

mihaisc commented Jun 14, 2021 •

edited

Loading

kremalicious commented Jun 14, 2021 •

edited

Loading

claudiaHash commented Jun 15, 2021 •

edited

Loading