Skip to content
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

Feature/cclow search filter #202

Merged
merged 26 commits into from
Dec 13, 2019
Merged

Feature/cclow search filter #202

merged 26 commits into from
Dec 13, 2019

Conversation

batraisk
Copy link

No description provided.

@simaob
Copy link
Contributor

simaob commented Dec 13, 2019

Are the filters hooked with the backend? They don't seem to be. Selecting a country doesn't filter the list of laws.

@tsubik
Copy link
Contributor

tsubik commented Dec 13, 2019

I think that's my bad, I saw a region before, missed the country. So, that's filtering by region only.

@tsubik
Copy link
Contributor

tsubik commented Dec 13, 2019

Taggin wasn't cleaned up before import, crap :/

image

@tsubik
Copy link
Contributor

tsubik commented Dec 13, 2019

or the delimiter should be ';' or maybe there were 2 delimiters :P

@simaob
Copy link
Contributor

simaob commented Dec 13, 2019

Yes there's some crap in there. But most of mine look good. I can sort it out in the prod console later =D split and save. =)

@tsubik
Copy link
Contributor

tsubik commented Dec 13, 2019

When filtering by Tags there should be AND not OR right?

@tsubik
Copy link
Contributor

tsubik commented Dec 13, 2019

or maybe OR

@simaob
Copy link
Contributor

simaob commented Dec 13, 2019

that's always a tough question... @faustoperez what do you think? Tags are "has this AND that" or "has this OR that"?

@tsubik
Copy link
Contributor

tsubik commented Dec 13, 2019

I see I must removed filter by geography.

@tsubik
Copy link
Contributor

tsubik commented Dec 13, 2019

Countries should be OR I guess, so maybe tags too for consitency.

@tsubik
Copy link
Contributor

tsubik commented Dec 13, 2019

I'm gonna bring back the geography filter @batraisk

@simaob
Copy link
Contributor

simaob commented Dec 13, 2019

Agreed, good thinking. OR it is.

@tsubik
Copy link
Contributor

tsubik commented Dec 13, 2019

Done, geography should work.

@simaob
Copy link
Contributor

simaob commented Dec 13, 2019

Any other feedback? It looks pretty good to me! =)

Copy link
Contributor

@tsubik tsubik left a comment

Choose a reason for hiding this comment

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

Looks great for me too! Great work @batraisk

From my side, that optimization, and checking trigrams to search more like fuzzy match could be next step

@simaob
Copy link
Contributor

simaob commented Dec 13, 2019

Yes, sounds good! Can come in a separate PR!

@simaob simaob merged commit 1e2e26a into develop Dec 13, 2019
@simaob simaob deleted the feature/cclow-searh-filter branch December 13, 2019 12:22
@tsubik
Copy link
Contributor

tsubik commented Dec 13, 2019

Let's check how it works on staging.

@batraisk
Copy link
Author

thank you @tsubik :)

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