Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Allow fuzzy search on data filter when getting transactions - Closes #2404 #2513

Merged
merged 11 commits into from
Nov 8, 2018
Merged

Allow fuzzy search on data filter when getting transactions - Closes #2404 #2513

merged 11 commits into from
Nov 8, 2018

Conversation

diego-G
Copy link

@diego-G diego-G commented Oct 31, 2018 •
edited
Loading

What was the problem?

Query was returning only exact matches.

How did I fix it?

Changing the subquery to support LIKE matches and removing null characters from the string to avoid errors.

How to test it?

mocha test/functional/http/get/transactions.js

Review checklist

@diego-G diego-G self-assigned this Oct 31, 2018
@diego-G diego-G requested a review from nazarhussain October 31, 2018 10:28
@diego-G diego-G requested review from 4miners and vitaly-t October 31, 2018 10:45
vitaly-t
vitaly-t reviewed Oct 31, 2018
View reviewed changes
@diego-G diego-G requested a review from vitaly-t October 31, 2018 16:35
@MaciejBaj MaciejBaj changed the title Allow fuzzy search on data filter when getting transactions - Close #2404 Allow fuzzy search on data filter when getting transactions - Closes #2404 Oct 31, 2018
vitaly-t
vitaly-t previously approved these changes Oct 31, 2018
View reviewed changes
Copy link
Contributor

@vitaly-t vitaly-t left a comment

Choose a reason for hiding this comment

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

LGTM.

@diego-G diego-G requested review from MaciejBaj and removed request for 4miners November 1, 2018 09:25
MaciejBaj
MaciejBaj previously approved these changes Nov 2, 2018
View reviewed changes
@diego-G diego-G requested a review from 4miners November 5, 2018 08:46
4miners
4miners previously approved these changes Nov 6, 2018
View reviewed changes
Copy link
Contributor

@4miners 4miners left a comment

Choose a reason for hiding this comment

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

The code looks fine. However, I'm not sure about the feature itself. What is the point of allowing wildcard search on this field? I don't really see use cases for it.

@nazarhussain nazarhussain mentioned this pull request Nov 6, 2018
Closed
@diego-G diego-G dismissed stale reviews from 4miners, MaciejBaj, and vitaly-t via 73b5d20 November 6, 2018 14:12
@diego-G diego-G requested review from 4miners and MaciejBaj November 6, 2018 14:14
4miners
4miners reviewed Nov 6, 2018
View reviewed changes
@diego-G diego-G force-pushed the 2404-allow_fuzzy_search_on_data_filter branch from f6f5652 to 90e2f81 Compare November 6, 2018 14:39
@diego-G diego-G requested a review from 4miners November 6, 2018 14:41
4miners
4miners reviewed Nov 6, 2018
View reviewed changes
@diego-G diego-G requested a review from 4miners November 6, 2018 14:56
4miners
4miners previously approved these changes Nov 7, 2018
View reviewed changes
nazarhussain
nazarhussain previously approved these changes Nov 8, 2018
View reviewed changes
@nazarhussain @diego-G
Add clarification on maxLength for additional data field
3c06042 
Co-Authored-By: diego-G <diego@lightcurve.io>
@diego-G diego-G dismissed stale reviews from nazarhussain and 4miners via 3c06042 November 8, 2018 09:09
@MaciejBaj MaciejBaj merged commit b922930 into development Nov 8, 2018
@MaciejBaj MaciejBaj deleted the 2404-allow_fuzzy_search_on_data_filter branch November 8, 2018 15:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nazarhussain nazarhussain nazarhussain approved these changes

@MaciejBaj MaciejBaj MaciejBaj approved these changes

@4miners 4miners 4miners approved these changes

@vitaly-t vitaly-t vitaly-t left review comments

Assignees

@diego-G diego-G

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow to search for transactions by data field
6 participants
@diego-G @nazarhussain @vitaly-t @MaciejBaj @4miners @karmacoma