Skip to content

Commit 4b44ffb

Browse files
bd-vaibhavPriya5
authored andcommitted
OTWO-7667 People page Query optimization (#1905)
* OTWO-7667 People page Query optimization * OTWO-7667 Fixed test errors for people page * OTWO-7667 Cache changes in people page * OTWO-7667 Removed ApplicationHelper from people controller
1 parent 6fc6886 commit 4b44ffb

8 files changed

Lines changed: 45 additions & 14 deletions

File tree

app/controllers/people_controller.rb

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,21 @@ def rankings; end
1111

1212
private
1313

14+
def only_device?
15+
request.env['HTTP_USER_AGENT']&.match?(/(iPhone|BlackBerry|Android)/)
16+
end
17+
1418
def query_or_cache_exist
15-
params[:query].blank? && Rails.cache.exist?('people_index_page') && Rails.cache.exist?('people_index_page_device')
19+
key = only_device? ? 'people_index_page_device' : 'people_index_page'
20+
params[:query].blank? && Rails.cache.exist?(key)
1621
end
1722

1823
def find_index_people
1924
@claimed_people = Person.find_claimed(params[:query], 'relevance')
20-
.preload(account: :markup)
2125
.paginate(page: 1, per_page: 3)
22-
@unclaimed_people = unclaimed_people(params[:query], 'relevance', 3)
23-
@unclaimed_people_count = Person::Count.unclaimed_by(params[:query], 'relevance')
26+
@unclaimed_people = unclaimed_people(params[:query], 'relevance', 4)
27+
@more_unclaimed = @unclaimed_people.length > 3
28+
@unclaimed_people = @unclaimed_people.first(3)
2429
end
2530

2631
def preload_data

app/controllers/unclaimed_controller.rb

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,17 @@ def find_emails(email_ids)
3838
end
3939

4040
def unclaimed_people(query, find_by, per_page = 10)
41-
name_ids = Person.joins(project: :best_analysis).unclaimed_people(q: query,
42-
find_by: find_by).limit(per_page).pluck(:name_id)
41+
name_ids = Person.unclaimed_people(q: query, find_by: find_by)
42+
.joins(:project)
43+
.where.not(projects: { best_analysis_id: nil })
44+
.limit(per_page).pluck(:name_id)
4345
unclaimed_people_with_limit(name_ids)
4446
end
4547

4648
# NOTE: Since this approach avoids the *includes*, it takes 3x DB time. However this prevents memory hog.
4749
def unclaimed_people_with_limit(unclaimed_name_ids)
4850
unclaimed_name_ids.map do |name_id|
49-
[name_id, Person.include_relations_and_order_by_kudo_position_and_name(name_id).limit(UNCLAIMED_TILE_LIMIT)]
51+
[name_id, Person.include_relations_and_order_by_kudo_position_and_name(name_id).limit(UNCLAIMED_TILE_LIMIT).to_a]
5052
end
5153
end
5254
end

app/models/account.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,11 @@ def edit_count
5252
end
5353

5454
def best_account_analysis
55-
AccountAnalysis.find_by(id: best_vita_id, account_id: id) || NilAccountAnalysis.new
55+
if association(:best_account_analysis).loaded?
56+
super || NilAccountAnalysis.new
57+
else
58+
AccountAnalysis.find_by(id: best_vita_id, account_id: id) || NilAccountAnalysis.new
59+
end
5660
end
5761

5862
def email_topics?

app/models/person.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ def find_claimed(query, sort_by)
4949
sort_by = sort_by.eql?('kudo_position') ? 'sort_by_kudo_position' : nil
5050
tsearch(query, sort_by)
5151
.includes(:account)
52-
.preload(account: :markup)
52+
.preload(account: [:markup, :projects, { best_account_analysis: :name_fact }])
5353
.where.not(account_id: nil)
5454
.references(:all)
5555
end
@@ -70,8 +70,8 @@ def rebuild_by_project_id(project_id)
7070
end
7171

7272
def include_relations_and_order_by_kudo_position_and_name(name_id)
73-
joins(project: :best_analysis).includes({ project: [{ best_analysis: :main_language }, :logo] }, :name,
74-
name_fact: :primary_language)
73+
joins(project: :best_analysis).preload({ project: [{ best_analysis: :main_language }, :logo] }, :name,
74+
name_fact: :primary_language)
7575
.where(name_id: name_id)
7676
.order(Arel.sql('COALESCE(kudo_position, 999999999), lower(effective_name)'))
7777
end

app/views/people/_claimed_person.html.haml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@
1616
.pull-left.account_details
1717
%h3
1818
%a.margin_top_20{ href: account_path(account), title: account.name }= truncate(account.name, length: 18)
19-
- markup_raw = account.markup ? account.markup.raw : ''
20-
%p.small.word_break_all= truncate(Markup.create(raw: markup_raw).first_line, length: 75)
19+
%p.small.word_break_all= truncate(account.markup&.first_line.to_s, length: 75)
2120
.small.member_info
2221
- if account.projects.any?
2322
%p

app/views/people/_people.html.haml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@
99
%h3.margin_bottom_15= t('.unclaimed_committer_ids')
1010
- @unclaimed_people.each do |_, people|
1111
= render partial: 'unclaimed_person', locals: { people: people }
12-
- if @unclaimed_people_count > 3
12+
- if @more_unclaimed
1313
%p
1414
%a.btn.btn-mini.btn-primary#more_unclaimed{ href: committers_path(query: params[:query]) }= t('.more_commiters')
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# frozen_string_literal: true
2+
3+
class AddIndexToPeopleNameIdKudoPosition < ActiveRecord::Migration[6.1]
4+
disable_ddl_transaction!
5+
6+
def change
7+
add_index :people, %i[name_id kudo_position],
8+
where: 'name_id IS NOT NULL',
9+
name: 'index_people_name_id_kudo_position',
10+
algorithm: :concurrently
11+
end
12+
end

db/structure.sql

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20539,6 +20539,13 @@ CREATE INDEX index_people_gin ON oh.people USING gin (vector);
2053920539
CREATE INDEX index_people_name_id ON oh.people USING btree (name_id) WHERE (name_id IS NOT NULL);
2054020540

2054120541

20542+
--
20543+
-- Name: index_people_name_id_kudo_position; Type: INDEX; Schema: oh; Owner: -
20544+
--
20545+
20546+
CREATE INDEX index_people_name_id_kudo_position ON oh.people USING btree (name_id, kudo_position) WHERE (name_id IS NOT NULL);
20547+
20548+
2054220549
--
2054320550
-- Name: index_people_on_account_id; Type: INDEX; Schema: oh; Owner: -
2054420551
--
@@ -28227,3 +28234,5 @@ INSERT INTO oh.schema_migrations (version) VALUES ('98');
2822728234

2822828235
INSERT INTO oh.schema_migrations (version) VALUES ('99');
2822928236

28237+
28238+
INSERT INTO oh.schema_migrations (version) VALUES ('20260603065929');

0 commit comments

Comments
 (0)