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 more relevant search results #4848

Merged

Conversation

shubhscoder
Copy link
Contributor

@shubhscoder shubhscoder commented Feb 18, 2019

Fixes #4823

  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts 📁
  • PR is descriptively titled 📑
  • screenshots/GIFs are attached 📎 in case of UI updation
  • ask @publiclab/reviewers for help, in a comment below

Thank you so much!

@plotsbot
Copy link
Collaborator

plotsbot commented Feb 18, 2019

1 Message
📖 @shubhscoder Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.

Generated by 🚫 Danger

@shubhscoder shubhscoder changed the title [WIP]Add more relevant search results Add more relevant search results Feb 20, 2019
@shubhscoder
Copy link
Contributor Author

@jywarren need your reviews on this ! I was thinking of adding a few more tests for individual nodes like tags, wikis ,etc. Rest everything looks fine to me. What do you think? Thank you so much!

Copy link
Member

@jywarren jywarren left a comment

Choose a reason for hiding this comment

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

This is awesome, thanks for your great work on this! I love the tests! Added a few suggestions.


def add_extra_results_for_transformed_queries(type)
search_type_object = ExecuteSearch.new.by(type, @search_criteria)
@additional_search_querys.each do |added_search_criteria|
Copy link
Member

Choose a reason for hiding this comment

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

This is cool! Could we pass this into the above functions by returning the value of additional_search_querys from this method, instead of using an instance variable? Thanks!

Copy link
Contributor Author

@shubhscoder shubhscoder Feb 21, 2019

Choose a reason for hiding this comment

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

@jywarren , I think its necessary to have additional_search_querys as a object variable because we are using it in all_content as well . Correct me if I am wrong. Thank you so much!

Copy link
Member

Choose a reason for hiding this comment

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

Oh! Well, could we pass it into there too by parameter? Let me look in a bit, thanks!

@@ -5,4 +5,13 @@ def lemmatize(word)
lem = Lemmatizer.new
lem.lemma(word)
end

def non_hyphenate_query(word)
word.gsub('-','')
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind using two-space indents here? It helps us keep the code tidy!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did that. Thank you!

@@ -77,4 +77,19 @@ class SearchControllerTest < ActionController::TestCase
assert_equal nodes_with_trawl, nodes_with_trawls
assert_response :success
end

test "search for purple-air also includes results for purpleair and vice versa" do
Copy link
Member

Choose a reason for hiding this comment

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

Let's also add "for hyphenated searches" just so it's easy to find the word hyphen when searching the codebase!

@@ -0,0 +1 @@
noun purpleair purple-air
Copy link
Member

Choose a reason for hiding this comment

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

Do you think we should just strip /all/ hyphens and provide them as related terms? We could do that with Regex! (https://rubular.org) and 'string'.gsub(/.../)

We might rename this and use it for more complex additional terms like h2s: hydrogen-sulfide as in the original issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jywarren sorry But I am not getting what you are trying to say. Also the link you provided is not loading

😅

Copy link
Member

Choose a reason for hiding this comment

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

Oops, http://rubular.com/ ! I meant we could do 'string'.gsub('-', '') to remove hyphens pretty easily. I clarified in #4823 in a longer comment about the 3 cases, i hope that helps!

Copy link
Member

Choose a reason for hiding this comment

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

OK, but so can we do one more thing and provide a second dict that's for non-hyphen-related transforms like in my comment:

h2s: hydrogen-sulfide

Otherwise this PR is good to merge!

Copy link
Member

Choose a reason for hiding this comment

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

I mean that we can either have one file specifically for "adding hyphens in" and another for "related terms" (even though both work similarly) OR we should simply consider "adding hyphens" to be a subset of "related terms" and have a single file. I'm fine with either, just don't want to have to ask people to put hyphen-unrelated word pairs into a file that's labelled with relation to hyphens, make sense?

Great!!!!

@shubhscoder
Copy link
Contributor Author

@jywarren sorry for the delay, I have modified the PR according to our discussion in #4823 . Please let me know if we need some more changes. Thank you so much!

@shubhscoder
Copy link
Contributor Author

@jywarren

I mean that we can either have one file specifically for "adding hyphens in" and another for "related terms" (even though both work similarly) OR we should simply consider "adding hyphens" to be a subset of "related terms" and have a single file. I'm fine with either, just don't want to have to ask people to put hyphen-unrelated word pairs into a file that's labelled with relation to hyphens, make sense?

We can have one file? We can name it related_terms and that takes care of hyphenated terms as well as related terms, whats your suggestion on this?

@jywarren
Copy link
Member

jywarren commented Mar 5, 2019

Sure one file sounds good. Ready then? This is exciting!!!!!!

@shubhscoder
Copy link
Contributor Author

Yes @jywarren , I think this is ready! Thank you!

@jywarren jywarren merged commit 984066a into publiclab:master Mar 6, 2019
@jywarren
Copy link
Member

jywarren commented Mar 6, 2019

Congratulations! Great work!!!! 🎉 🎉

@jywarren
Copy link
Member

jywarren commented Mar 6, 2019

Shall we post a "call for related terms" now, in a new issue, where we can collect related term pairs?

@shubhscoder
Copy link
Contributor Author

Yes @jywarren . I will open the issue now, you can suggest some changes there if required.
Thank you so much!

jywarren pushed a commit that referenced this pull request Mar 9, 2019
* Added debounce for typeahead search optimization

* Update README.md (#4883)

The "What makes this project different" section had several long sentences which were difficult to understand. I tried to fix this by breaking the sentences down into smaller more concise sentences.

* convert chars to unicode (#4901)

* fixes for map module (#4909)

* fixes for map module

* yarn.lock update

* updated yarn.lock (#4911)

* updated yarn.lock

* tweak

* Remove useless variable assignment (#4885)

* Simplify username generation

* Use status module

* Tiny fix

* Fix indentation

* Made moderate buttons to appear on the same line (#4913)

* Made moderate buttons to appear on the same line

* Removed break tag.

* Clean up (#4902)

* Clean up with Rubocop

* More cleanup

* Excluse views

* Small refactor

* More clean up

* Clean up

* Fix conflict

* Tiny fix

* Follow tag asynchronously (#4587)

* Follow tag asynchronously


Added link to manage subscriptions


Add precondition failed http error code


cleanup


codeclimate fix


Fix typo change


remove data-remote


Preserve code for non xhr requests


Seperate JS and html


Codeclimate fix


Code Climate fixes


Updated tests

* Remove pre-compilation of async.js

* pre compilation of js assets

* cc fix try

* Move verification link (#4786)

* Move verification link

* Change link to text

* Remove unnecessary message

* popover works, styling is still not showing under elements but is und… (#4906)

* popover works, styling is still not showing under elements but is under.btn-default

* copied yml file back in

* First timer tag script (#4878)

* First timer tag script

* migration for attaching first-time-poster

* change migration bump version to 5.1

* Update schema.rb.example

* Consolidating ranges and stats (#4887)

* restyle range page to be a partial

* remove static stats

* merge range and main stats

* render range in main stats

* fix failing range test

* add questions vs answers graph

* add go back and further buttons

* minor fixes

* Change contribution graph making method

Change method to create graph based on the range given

* fix failing test and code climate issues

* remove commented code and indent

* remove stats_nav partial

* add contribution graph tests

* review requests implemetation

* move tag graph button (#4921)

* move tag graph button

* Update _user_controls.html.erb

* Added reply by tweet feature (#3175)

* Added reply by tweet feature

* Updated schedule.rb file

* Finalized reply-by-tweet

* Corrected schema version

* Minor change

* Added reply_by_tweet doc

* Minor changes

* Added twitter gem

* Minor changes

* Added Environment variables in Docker

* Added summery in Doc file

* Corrected schema version

* Added some documentation

* Added some documentation

* Migration timestamp changed

* Changed migration

* Minor change

* Minor changes

* Added rake to general gem list

* Added bundle exec for rake/rails tasks in schedule.rb

* Added path env variable

* MINOR CHANGE

* MINOR CHANGE

* MINOR CHANGE

* MINOR CHANGE

* Added print statement to check print

* Minor change

* Minor change

* Changed whenever config

* Minor change

* Completed reply by tweet feature

* Minor change

* Minor change

* Added gemfile.lock

* Update comment.rb

* Added gemfile.lock

* Minor changes

* Minor changes

* Minor changes

* Minor changes

* Add more relevant search results (#4848)

* add helper functions

* Modify controller to take extra results

* add dict file

* add  more objects

* refactor code and add tests

* cc fix

* change numbers to account for additions in fixtures

* fix tests

* remove unused include

* code quality fixes

* tab fix

* changing implementation ideas, removal of unnecessary code

* newline fix

* cc fix

* cc space inside brackets fix

* modify query to get rid of redundant words

* reduce word to root then tranform

* Change file name

* chore: update README.md (#4926)

Fixes task mentioned in issue #4749

* Bump cytoscape from 3.4.2 to 3.5.0 (#4929)

Bumps [cytoscape](https://github.com/cytoscape/cytoscape.js) from 3.4.2 to 3.5.0.
- [Release notes](https://github.com/cytoscape/cytoscape.js/releases)
- [Commits](cytoscape/cytoscape.js@v3.4.2...v3.5.0)

Signed-off-by: dependabot[bot] <support@dependabot.com>

* Tiny fix (#4933)

* Tiny fix

* Rubocop

* Update 20190301075323_add_first_tag_poster.rb

* updates (#4851)

* fix for wiki update (#4942)

* fixes, extend to title suggestions

* extended to atwho
@jywarren
Copy link
Member

jywarren commented Mar 9, 2019

Hi @shubhscoder - i also noticed that in Travis each word is being output into the logs. Is there a way to turn that off? Just curious!

@jywarren
Copy link
Member

jywarren commented Mar 9, 2019

@shubhscoder
Copy link
Contributor Author

@jywarren , I am not sure of this, I will open a issue in the gem repository, maybe we could get some help there. Thank you!

icarito pushed a commit to icarito/plots2 that referenced this pull request Apr 9, 2019
* add helper functions

* Modify controller to take extra results

* add dict file

* add  more objects

* refactor code and add tests

* cc fix

* change numbers to account for additions in fixtures

* fix tests

* remove unused include

* code quality fixes

* tab fix

* changing implementation ideas, removal of unnecessary code

* newline fix

* cc fix

* cc space inside brackets fix

* modify query to get rid of redundant words

* reduce word to root then tranform

* Change file name
icarito pushed a commit to icarito/plots2 that referenced this pull request Apr 9, 2019
* Added debounce for typeahead search optimization

* Update README.md (publiclab#4883)

The "What makes this project different" section had several long sentences which were difficult to understand. I tried to fix this by breaking the sentences down into smaller more concise sentences.

* convert chars to unicode (publiclab#4901)

* fixes for map module (publiclab#4909)

* fixes for map module

* yarn.lock update

* updated yarn.lock (publiclab#4911)

* updated yarn.lock

* tweak

* Remove useless variable assignment (publiclab#4885)

* Simplify username generation

* Use status module

* Tiny fix

* Fix indentation

* Made moderate buttons to appear on the same line (publiclab#4913)

* Made moderate buttons to appear on the same line

* Removed break tag.

* Clean up (publiclab#4902)

* Clean up with Rubocop

* More cleanup

* Excluse views

* Small refactor

* More clean up

* Clean up

* Fix conflict

* Tiny fix

* Follow tag asynchronously (publiclab#4587)

* Follow tag asynchronously


Added link to manage subscriptions


Add precondition failed http error code


cleanup


codeclimate fix


Fix typo change


remove data-remote


Preserve code for non xhr requests


Seperate JS and html


Codeclimate fix


Code Climate fixes


Updated tests

* Remove pre-compilation of async.js

* pre compilation of js assets

* cc fix try

* Move verification link (publiclab#4786)

* Move verification link

* Change link to text

* Remove unnecessary message

* popover works, styling is still not showing under elements but is und… (publiclab#4906)

* popover works, styling is still not showing under elements but is under.btn-default

* copied yml file back in

* First timer tag script (publiclab#4878)

* First timer tag script

* migration for attaching first-time-poster

* change migration bump version to 5.1

* Update schema.rb.example

* Consolidating ranges and stats (publiclab#4887)

* restyle range page to be a partial

* remove static stats

* merge range and main stats

* render range in main stats

* fix failing range test

* add questions vs answers graph

* add go back and further buttons

* minor fixes

* Change contribution graph making method

Change method to create graph based on the range given

* fix failing test and code climate issues

* remove commented code and indent

* remove stats_nav partial

* add contribution graph tests

* review requests implemetation

* move tag graph button (publiclab#4921)

* move tag graph button

* Update _user_controls.html.erb

* Added reply by tweet feature (publiclab#3175)

* Added reply by tweet feature

* Updated schedule.rb file

* Finalized reply-by-tweet

* Corrected schema version

* Minor change

* Added reply_by_tweet doc

* Minor changes

* Added twitter gem

* Minor changes

* Added Environment variables in Docker

* Added summery in Doc file

* Corrected schema version

* Added some documentation

* Added some documentation

* Migration timestamp changed

* Changed migration

* Minor change

* Minor changes

* Added rake to general gem list

* Added bundle exec for rake/rails tasks in schedule.rb

* Added path env variable

* MINOR CHANGE

* MINOR CHANGE

* MINOR CHANGE

* MINOR CHANGE

* Added print statement to check print

* Minor change

* Minor change

* Changed whenever config

* Minor change

* Completed reply by tweet feature

* Minor change

* Minor change

* Added gemfile.lock

* Update comment.rb

* Added gemfile.lock

* Minor changes

* Minor changes

* Minor changes

* Minor changes

* Add more relevant search results (publiclab#4848)

* add helper functions

* Modify controller to take extra results

* add dict file

* add  more objects

* refactor code and add tests

* cc fix

* change numbers to account for additions in fixtures

* fix tests

* remove unused include

* code quality fixes

* tab fix

* changing implementation ideas, removal of unnecessary code

* newline fix

* cc fix

* cc space inside brackets fix

* modify query to get rid of redundant words

* reduce word to root then tranform

* Change file name

* chore: update README.md (publiclab#4926)

Fixes task mentioned in issue publiclab#4749

* Bump cytoscape from 3.4.2 to 3.5.0 (publiclab#4929)

Bumps [cytoscape](https://github.com/cytoscape/cytoscape.js) from 3.4.2 to 3.5.0.
- [Release notes](https://github.com/cytoscape/cytoscape.js/releases)
- [Commits](cytoscape/cytoscape.js@v3.4.2...v3.5.0)

Signed-off-by: dependabot[bot] <support@dependabot.com>

* Tiny fix (publiclab#4933)

* Tiny fix

* Rubocop

* Update 20190301075323_add_first_tag_poster.rb

* updates (publiclab#4851)

* fix for wiki update (publiclab#4942)

* fixes, extend to title suggestions

* extended to atwho
SrinandanPai pushed a commit to SrinandanPai/plots2 that referenced this pull request May 5, 2019
* add helper functions

* Modify controller to take extra results

* add dict file

* add  more objects

* refactor code and add tests

* cc fix

* change numbers to account for additions in fixtures

* fix tests

* remove unused include

* code quality fixes

* tab fix

* changing implementation ideas, removal of unnecessary code

* newline fix

* cc fix

* cc space inside brackets fix

* modify query to get rid of redundant words

* reduce word to root then tranform

* Change file name
SrinandanPai pushed a commit to SrinandanPai/plots2 that referenced this pull request May 5, 2019
* Added debounce for typeahead search optimization

* Update README.md (publiclab#4883)

The "What makes this project different" section had several long sentences which were difficult to understand. I tried to fix this by breaking the sentences down into smaller more concise sentences.

* convert chars to unicode (publiclab#4901)

* fixes for map module (publiclab#4909)

* fixes for map module

* yarn.lock update

* updated yarn.lock (publiclab#4911)

* updated yarn.lock

* tweak

* Remove useless variable assignment (publiclab#4885)

* Simplify username generation

* Use status module

* Tiny fix

* Fix indentation

* Made moderate buttons to appear on the same line (publiclab#4913)

* Made moderate buttons to appear on the same line

* Removed break tag.

* Clean up (publiclab#4902)

* Clean up with Rubocop

* More cleanup

* Excluse views

* Small refactor

* More clean up

* Clean up

* Fix conflict

* Tiny fix

* Follow tag asynchronously (publiclab#4587)

* Follow tag asynchronously


Added link to manage subscriptions


Add precondition failed http error code


cleanup


codeclimate fix


Fix typo change


remove data-remote


Preserve code for non xhr requests


Seperate JS and html


Codeclimate fix


Code Climate fixes


Updated tests

* Remove pre-compilation of async.js

* pre compilation of js assets

* cc fix try

* Move verification link (publiclab#4786)

* Move verification link

* Change link to text

* Remove unnecessary message

* popover works, styling is still not showing under elements but is und… (publiclab#4906)

* popover works, styling is still not showing under elements but is under.btn-default

* copied yml file back in

* First timer tag script (publiclab#4878)

* First timer tag script

* migration for attaching first-time-poster

* change migration bump version to 5.1

* Update schema.rb.example

* Consolidating ranges and stats (publiclab#4887)

* restyle range page to be a partial

* remove static stats

* merge range and main stats

* render range in main stats

* fix failing range test

* add questions vs answers graph

* add go back and further buttons

* minor fixes

* Change contribution graph making method

Change method to create graph based on the range given

* fix failing test and code climate issues

* remove commented code and indent

* remove stats_nav partial

* add contribution graph tests

* review requests implemetation

* move tag graph button (publiclab#4921)

* move tag graph button

* Update _user_controls.html.erb

* Added reply by tweet feature (publiclab#3175)

* Added reply by tweet feature

* Updated schedule.rb file

* Finalized reply-by-tweet

* Corrected schema version

* Minor change

* Added reply_by_tweet doc

* Minor changes

* Added twitter gem

* Minor changes

* Added Environment variables in Docker

* Added summery in Doc file

* Corrected schema version

* Added some documentation

* Added some documentation

* Migration timestamp changed

* Changed migration

* Minor change

* Minor changes

* Added rake to general gem list

* Added bundle exec for rake/rails tasks in schedule.rb

* Added path env variable

* MINOR CHANGE

* MINOR CHANGE

* MINOR CHANGE

* MINOR CHANGE

* Added print statement to check print

* Minor change

* Minor change

* Changed whenever config

* Minor change

* Completed reply by tweet feature

* Minor change

* Minor change

* Added gemfile.lock

* Update comment.rb

* Added gemfile.lock

* Minor changes

* Minor changes

* Minor changes

* Minor changes

* Add more relevant search results (publiclab#4848)

* add helper functions

* Modify controller to take extra results

* add dict file

* add  more objects

* refactor code and add tests

* cc fix

* change numbers to account for additions in fixtures

* fix tests

* remove unused include

* code quality fixes

* tab fix

* changing implementation ideas, removal of unnecessary code

* newline fix

* cc fix

* cc space inside brackets fix

* modify query to get rid of redundant words

* reduce word to root then tranform

* Change file name

* chore: update README.md (publiclab#4926)

Fixes task mentioned in issue publiclab#4749

* Bump cytoscape from 3.4.2 to 3.5.0 (publiclab#4929)

Bumps [cytoscape](https://github.com/cytoscape/cytoscape.js) from 3.4.2 to 3.5.0.
- [Release notes](https://github.com/cytoscape/cytoscape.js/releases)
- [Commits](cytoscape/cytoscape.js@v3.4.2...v3.5.0)

Signed-off-by: dependabot[bot] <support@dependabot.com>

* Tiny fix (publiclab#4933)

* Tiny fix

* Rubocop

* Update 20190301075323_add_first_tag_poster.rb

* updates (publiclab#4851)

* fix for wiki update (publiclab#4942)

* fixes, extend to title suggestions

* extended to atwho
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