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
AO3-5226: People Search #3191
Conversation
elzj
added some commits
Oct 20, 2017
| @@ -0,0 +1,121 @@ | ||
| +require 'spec_helper' | ||
| + | ||
| +describe PseudDecorator do |
| + before(:all) do | ||
| + @pseud = create(:pseud) | ||
| + @search_results = [{ | ||
| + "_id"=>"#{@pseud.id}", |
houndci-bot
Dec 1, 2017
Surrounding space missing for operator =>.
Prefer to_s over string interpolation.
| + @pseud = create(:pseud) | ||
| + @search_results = [{ | ||
| + "_id"=>"#{@pseud.id}", | ||
| + "_source"=>{ |
| + @search_results = [{ | ||
| + "_id"=>"#{@pseud.id}", | ||
| + "_source"=>{ | ||
| + "id"=>@pseud.id, |
| + "_id"=>"#{@pseud.id}", | ||
| + "_source"=>{ | ||
| + "id"=>@pseud.id, | ||
| + "user_id"=>@pseud.user_id, |
| + "_source"=>{ | ||
| + "id"=>@pseud.id, | ||
| + "user_id"=>@pseud.user_id, | ||
| + "name"=>@pseud.name, |
| + "id"=>@pseud.id, | ||
| + "user_id"=>@pseud.user_id, | ||
| + "name"=>@pseud.name, | ||
| + "description"=>nil, |
| + "user_id"=>@pseud.user_id, | ||
| + "name"=>@pseud.name, | ||
| + "description"=>nil, | ||
| + "user_login"=>@pseud.user_login, |
| + "name"=>@pseud.name, | ||
| + "description"=>nil, | ||
| + "user_login"=>@pseud.user_login, | ||
| + "byline"=>@pseud.byline, |
| + "description"=>nil, | ||
| + "user_login"=>@pseud.user_login, | ||
| + "byline"=>@pseud.byline, | ||
| + "collection_ids"=>[1], |
| + "user_login"=>@pseud.user_login, | ||
| + "byline"=>@pseud.byline, | ||
| + "collection_ids"=>[1], | ||
| + "sortable_name"=>@pseud.name.downcase, |
| + "byline"=>@pseud.byline, | ||
| + "collection_ids"=>[1], | ||
| + "sortable_name"=>@pseud.name.downcase, | ||
| + "fandoms"=>[{"id"=>13, "name"=>"Stargate SG-1", "count"=>7}], |
houndci-bot
Dec 1, 2017
Surrounding space missing for operator =>.
Space inside { missing.
Space inside } missing.
| + "collection_ids"=>[1], | ||
| + "sortable_name"=>@pseud.name.downcase, | ||
| + "fandoms"=>[{"id"=>13, "name"=>"Stargate SG-1", "count"=>7}], | ||
| + "public_bookmarks_count"=>5, |
| + "sortable_name"=>@pseud.name.downcase, | ||
| + "fandoms"=>[{"id"=>13, "name"=>"Stargate SG-1", "count"=>7}], | ||
| + "public_bookmarks_count"=>5, | ||
| + "general_works_count"=>10, |
| + "fandoms"=>[{"id"=>13, "name"=>"Stargate SG-1", "count"=>7}], | ||
| + "public_bookmarks_count"=>5, | ||
| + "general_works_count"=>10, | ||
| + "public_works_count"=>7 |
| + end | ||
| + end | ||
| + | ||
| + context "with search data" do |
sarken
added
the
Awaiting review
label
Dec 1, 2017
| def search | ||
| - if params[:query].present? | ||
| + if use_new_search? | ||
| + new_search and return |
| @@ -0,0 +1,111 @@ | ||
| +class PseudDecorator < SimpleDelegator |
| @@ -0,0 +1,111 @@ | ||
| +class PseudDecorator < SimpleDelegator | ||
| + |
| + end | ||
| + | ||
| + def works_link | ||
| + return unless works_count > 0 |
| + end | ||
| + | ||
| + def bookmarks_link | ||
| + return unless bookmarks_count > 0 |
| @@ -106,6 +107,13 @@ def invalidate_bookmark_count | ||
| end | ||
| end | ||
| + # We index the bookmark count, so if it should change, update the pseud | ||
| + def update_pseud_index | ||
| + return unless $rollout.active?(:start_new_indexing) |
| @@ -25,7 +36,58 @@ def document(object) | ||
| object.as_json( | ||
| root: false, | ||
| only: [:id, :user_id, :name, :description, :created_at], | ||
| - methods: [:user_login] | ||
| - ) | ||
| + methods: [ |
| + # [{id: 1, name: "Star Trek", count: 5}] | ||
| + def tag_info(pseud, tag_type) | ||
| + pseud.direct_filters.where(works: countable_works_conditions). | ||
| + by_type(tag_type). |
| + def tag_info(pseud, tag_type) | ||
| + pseud.direct_filters.where(works: countable_works_conditions). | ||
| + by_type(tag_type). | ||
| + group_by(&:id). |
| + pseud.direct_filters.where(works: countable_works_conditions). | ||
| + by_type(tag_type). | ||
| + group_by(&:id). | ||
| + map{ |id, tags| { |
houndci-bot
Dec 1, 2017
Use 2 (not 21) spaces for indenting an expression spanning multiple lines.
Space missing to the left of {.
Avoid using {...} for multi-line blocks.
Block body expression is on the same line as the block start.
| + by_type(tag_type). | ||
| + group_by(&:id). | ||
| + map{ |id, tags| { | ||
| + id: id, |
houndci-bot
Dec 1, 2017
Use 2 spaces for indentation in a hash, relative to the start of the line where the left curly brace is.
| + map{ |id, tags| { | ||
| + id: id, | ||
| + name: tags.first.name, | ||
| + count: tags.length } |
houndci-bot
Dec 1, 2017
Closing hash brace must be on the line after the last hash element when opening brace is on a separate line from the first hash element.
| @@ -28,6 +28,14 @@ def collection_filter | ||
| { term: { collection_ids: options[:collection_id] } } if options[:collection_id] | ||
| end | ||
| + def fandom_filter | ||
| + if options[:fandom_ids] |
houndci-bot
Dec 1, 2017
Use safe navigation (&.) instead of checking if an object exists before calling the method.
| + names = @options[:fandom].split(',').map(&:squish) | ||
| + @options[:fandom_ids] = Tag.where(name: names).pluck(:id) | ||
| + end | ||
| + |
| + | ||
| + # Take the most direct route from tag to pseud and queue up to reindex | ||
| + def reindex_pseuds | ||
| + return unless $rollout.active?(:start_new_indexing) |
| + def reindex_pseuds | ||
| + return unless $rollout.active?(:start_new_indexing) | ||
| + Creatorship.select(:id, :pseud_id). | ||
| + joins("JOIN filter_taggings ON filter_taggings.filterable_id = creatorships.creation_id"). |
| + return unless $rollout.active?(:start_new_indexing) | ||
| + Creatorship.select(:id, :pseud_id). | ||
| + joins("JOIN filter_taggings ON filter_taggings.filterable_id = creatorships.creation_id"). | ||
| + where("filter_taggings.filter_id = ? AND filter_taggings.filterable_type = 'Work' AND creatorships.creation_type = 'Work'", id). |
| + Creatorship.select(:id, :pseud_id). | ||
| + joins("JOIN filter_taggings ON filter_taggings.filterable_id = creatorships.creation_id"). | ||
| + where("filter_taggings.filter_id = ? AND filter_taggings.filterable_type = 'Work' AND creatorships.creation_type = 'Work'", id). | ||
| + find_in_batches do |batch| |
| @@ -255,6 +255,12 @@ def expire_caches | ||
| Work.expire_work_tag_groups_id(self.id) | ||
| end | ||
| + def update_pseud_index | ||
| + return unless $rollout.active?(:start_new_indexing) |
elzj
added some commits
Dec 1, 2017
| + { | ||
| + simple_query_string:{ | ||
| + query: escape_reserved_characters(options[:query]), | ||
| + fields: ["name^5", "user_login^2", "description"], |
redsummernight
Dec 2, 2017
Contributor
For my own reference, this means name is 5 times as important as description. https://www.elastic.co/guide/en/elasticsearch/reference/6.0/query-dsl-multi-match-query.html#_literal_fields_literal_and_per_field_boosting
| + users = User.where(id: pseuds.map(&:user_id)).group_by(&:id) | ||
| + work_counts | ||
| + bookmark_counts | ||
| + work_key = User.current_user.present? ? :general_works_count : :public_works_count |
sarken
Dec 4, 2017
Owner
I can never remember -- will User.current_user.present? be true if you're logged in as an admin? If not, should this be updated so admins will see the same work counts as users? (I know this isn't intended as an admin tool, but I have a feeling it will be useful at times.)
| + # We index the bookmark count, so if it should change, update the pseud | ||
| + def update_pseud_index | ||
| + return unless $rollout.active?(:start_new_indexing) | ||
| + return unless destroyed? || saved_change_to_id? || saved_change_to_private? |
| + methods: [ | ||
| + :user_login, | ||
| + :byline, | ||
| + :collection_ids |
elzj
Dec 4, 2017
Owner
Yes, it might not be very useful, but it still exists, so I thought we should keep it in the index.
| @@ -0,0 +1,27 @@ | ||
| +<%= form_for @search, as: :people_search, :url => search_people_path, :html => {:class => 'search', :method => :get} do |f| %> | ||
| + <fieldset> | ||
| + <legend>Search people</legend> |
sarken
Dec 5, 2017
Owner
Could you make this translatable and remove the colons from the field labels? Also, if you could convert it from hash rockets, that would be great
| @@ -1,49 +1,25 @@ | ||
| <!--SEARCHBROWSE Descriptive page name, messages and instructions--> | ||
| <div class="pseud"> |
sarken
Dec 5, 2017
Owner
Could you remove this surrounding div? There's no godly reason for it to be here -- I wouldn't be surprised if it were left over from layout 1.0
| + <%= f.text_field :fandom, autocomplete_options("fandom") %> | ||
| + </dd> | ||
| + </dl> | ||
| + <p class="submit actions"><%= f.submit ts('Search people'), :name => nil %></p> |
sarken
Dec 5, 2017
Owner
Could this be made title case (Search People) to go with our other buttons?
elzj commentedDec 1, 2017
•
Edited 1 time
-
elzj
Dec 2, 2017
Issue
https://otwarchive.atlassian.net/browse/AO3-5226
https://otwarchive.atlassian.net/browse/AO3-5151
Purpose
Updates people search to add an autocomplete field and a fandom search. Also simplifies some unnecessary people pages that we weren't really using.
Testing
See issue.