Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
Already on GitHub? Sign in to your account
Re-merge Elasticsearch branch #3179
Conversation
WendyBeth
and others
added some commits
Oct 20, 2017
| end | ||
| - describe "when searching restricted works" do | ||
| + describe '#search_results' do |
| + | ||
| + # ES UPGRADE TRANSITION # | ||
| + # Remove $rollout activation & unless block | ||
| + $rollout.activate :start_new_indexing |
| + # Remove $rollout activation & unless block | ||
| + $rollout.activate :start_new_indexing | ||
| + | ||
| + unless elasticsearch_enabled?($elasticsearch) |
| + $rollout.activate :start_new_indexing | ||
| + | ||
| + unless elasticsearch_enabled?($elasticsearch) | ||
| + $rollout.activate :stop_old_indexing |
| + | ||
| + unless elasticsearch_enabled?($elasticsearch) | ||
| + $rollout.activate :stop_old_indexing | ||
| + $rollout.activate :use_new_search |
| - Bookmark.tire.index.delete | ||
| - Bookmark.create_elasticsearch_index | ||
| + ['work', 'bookmark', 'pseud', 'tag'].each do |index| |
| +# ES UPGRADE TRANSITION # | ||
| +# Remove method | ||
| +def elasticsearch_enabled?(elasticsearch_instance) | ||
| + elasticsearch_instance.cluster.health rescue nil |
| +# ES UPGRADE TRANSITION # | ||
| +# Remove method | ||
| +def deprecate_old_elasticsearch_test | ||
| + deprecate_unless(elasticsearch_enabled?($elasticsearch)) do |
| +def update_and_refresh_indexes(klass_name) | ||
| + # ES UPGRADE TRANSITION # | ||
| + # Remove block | ||
| + if elasticsearch_enabled?($elasticsearch) |
| + indexer = indexer_class.new(klass_name.capitalize.constantize.all.pluck(:id)) | ||
| + indexer.index_documents if klass_name.capitalize.constantize.any? | ||
| + | ||
| + $new_elasticsearch.indices.refresh(index: "ao3_test_#{klass_name}s") |
| +end | ||
| + | ||
| +def refresh_index_without_updating(klass_name) | ||
| + $new_elasticsearch.indices.refresh(index: "ao3_test_#{klass_name}s") |
| +def delete_index(index) | ||
| + # ES UPGRADE TRANSITION # | ||
| + # Remove block | ||
| + if elasticsearch_enabled?($elasticsearch) |
| + | ||
| + # NEW ES | ||
| + index_name = "ao3_test_#{index}s" | ||
| + if $new_elasticsearch.indices.exists? index: index_name |
| + # NEW ES | ||
| + index_name = "ao3_test_#{index}s" | ||
| + if $new_elasticsearch.indices.exists? index: index_name | ||
| + $new_elasticsearch.indices.delete index: index_name |
| +end | ||
| + | ||
| +def delete_test_indices | ||
| + indices = $new_elasticsearch.indices.get_mapping.keys.select { |key| key.match("test") } |
| +def delete_test_indices | ||
| + indices = $new_elasticsearch.indices.get_mapping.keys.select { |key| key.match("test") } | ||
| + indices.each do |index| | ||
| + $new_elasticsearch.indices.delete(index: index) |
ariana-paris
reviewed
Nov 23, 2017
Made a few comments and ignored a bunch of single quotes introduced by the changes for no good reason
| - options.merge!(page: params[:page]) | ||
| + if params[:include_bookmark_search].present? |
ariana-paris
Nov 23, 2017
Contributor
Would this be an opportunity to use safe navigation (i.e.: params[:include_bookmark_search]&.keys.each) or would that not work in this case? If it would - same for the next block too and the similar code in the WorkController. Just to save some lines of code...
| - bookmarkable_type: { | ||
| - type: 'string', | ||
| - index: 'not_analyzed' | ||
| + "properties" => { |
ariana-paris
Nov 23, 2017
Contributor
I suppose this could use the new Ruby 1.9 syntax like the equivalent block in the Works indexer
| } | ||
| }, | ||
| - mappings: mapping | ||
| + mappings: mapping, |
| mapping do | ||
| indexes :id, index: :not_analyzed | ||
| - indexes :name, analyzer: 'snowball', boost: 100 | ||
| + indexes :name#, analyzer: 'snowball', boost: 100 |
ariana-paris
Nov 23, 2017
Contributor
I initially though the hash to comment out the snowball analyser was part of the name - maybe add a space so it's more obvious that it's starting a comment, or remove the commented out code altogether since this is the legacy code block anyway - though I'm not clear why we'd want to change it at this stage
| - audio_ids = [70308] # Podfic | ||
| - art_ids = [7844, 125758] # Fanart, Arts | ||
| + audio_ids = [70308, 1098169] # Podfic, Audio Content | ||
| + art_ids = [7844, 125758, 3863] # Fanart, Arts |
| +@no-txn @bookmarks @search | ||
| +Feature: Browse Bookmarks | ||
| + | ||
| + Scenario: Bookmarks appear on both the user's bookmark page and on the bookmark page for the pseud they used create the bookmark |
| When I follow "first.fandom" | ||
| Then I should see "This tag belongs to the Fandom Category" | ||
| - When I am on the search tags page | ||
| + # If this was a bug, it's fixed now? |
elzj commentedNov 22, 2017
Issue
https://otwarchive.atlassian.net/browse/AO3-4815 and others
Purpose
We had to back the code in progress out of master to address time-sensitive issues. This merges it all back together.