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

add postgres full text search by tokens #1696

Merged
merged 23 commits into from
Apr 16, 2019
Merged

Conversation

ayrat555
Copy link
Contributor

@ayrat555 ayrat555 commented Apr 3, 2019

resolves #1670
fixes #1745
fixes #403

img

Changelog:

  • add full-text search on tokens
  • replaces phantomjs with Chrome Headless for wallaby

@ghost ghost assigned ayrat555 Apr 3, 2019
@ghost ghost added the in progress label Apr 3, 2019
@coveralls
Copy link

coveralls commented Apr 3, 2019

Pull Request Test Coverage Report for Build 39592cff-fe6f-4861-be8f-9d0d41c303b5

  • 7 of 9 (77.78%) changed or added relevant lines in 3 files are covered.
  • 9 unchanged lines in 5 files lost coverage.
  • Overall coverage decreased (-0.2%) to 82.995%

Changes Missing Coverage Covered Lines Changed/Added Lines %
apps/block_scout_web/lib/block_scout_web/controllers/chain_controller.ex 3 5 60.0%
Files with Coverage Reduction New Missed Lines %
apps/block_scout_web/lib/block_scout_web/views/address_view.ex 1 90.0%
apps/block_scout_web/lib/block_scout_web/views/address_contract_view.ex 1 87.5%
apps/block_scout_web/lib/block_scout_web/controllers/address_contract_verification_controller.ex 1 90.0%
apps/block_scout_web/test/block_scout_web/features/pages/contract_verify_page.ex 2 75.0%
apps/block_scout_web/test/block_scout_web/features/pages/address_contract_page.ex 4 0.0%
Totals Coverage Status
Change from base Build a53a87c5-5c0f-459c-b7f7-e0b10a1b66d2: -0.2%
Covered Lines: 4412
Relevant Lines: 5316

💛 - Coveralls

@ayrat555 ayrat555 changed the title [WIP] add postgres full text search by tokens add postgres full text search by tokens Apr 15, 2019
Copy link
Contributor

@goodsoft goodsoft left a comment

Choose a reason for hiding this comment

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

Looks ok, just a couple of minor comments.
Also, please attach a screenshot of the feature.

script-src 'self' 'unsafe-inline' 'unsafe-eval';\
style-src 'self' 'unsafe-inline' 'unsafe-eval' https://fonts.googleapis.com;\
script-src 'self' 'unsafe-inline' 'unsafe-eval' http://nico-amsterdam.github.io;\
style-src 'self' 'unsafe-inline' 'unsafe-eval' https://fonts.googleapis.com http://nico-amsterdam.github.io;\
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we force HTTPS here?

.hide-not-found div.awesomplete .awe-not-found {border-color: lightblue}

div.awesomplete .awe-not-found {border: 2px solid red}
</style>
Copy link
Contributor

Choose a reason for hiding this comment

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

I see this code in the README of Awesomplete, but can't it be installed via NPM and imported from JS/CSS?
I don't like including scripts from third-party hosts, because if a script loads too slowly, the whole page will be loaded slowly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, it can't be installed from npm


assert Enum.count(json_response(conn, 200)) == 1
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably include one more matching and non-matching token for better coverage.

@ayrat555 ayrat555 merged commit 2a365f8 into master Apr 16, 2019
@ayrat555 ayrat555 deleted the ab-token-full-text-search branch April 16, 2019 08:50
@ghost ghost removed the in progress label Apr 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants