Skip to content

Commit

Permalink
Only SELECT person's roles columns that we need
Browse files Browse the repository at this point in the history
The idea with this change is to reduce the amount of data retrieved from
the database in an attempt to speed up the overall response time to the
GraphQL client.

So far we've implemented this optimisation for the root edition,
linked-to editions and reverse-linked editions. Here we're introducing
the change for the person's role_appointments/roles fields specific to
the ministers index type.

We've followed the patterns established in the previous implementations
with the big differences being mostly:
* the larger number of JOINs needed in this query because we're
  traversing 2 levels of links
* the convenience of only needing to handle link set links here because
  we know that those are the only types of links involved in linking the
  ministers index's person-role_appointments-roles

Because of the large number of JOINS, we've resorted to supplying SQL
strings in order to provide our own, hopefully clearer, join table
aliases as opposed to leaving that to Active Record to generate.
  • Loading branch information
mike3985 committed Mar 7, 2025
1 parent 0039ca0 commit 4dfce35
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 41 deletions.
101 changes: 64 additions & 37 deletions app/graphql/sources/person_current_roles_source.rb
Original file line number Diff line number Diff line change
@@ -1,51 +1,78 @@
module Sources
class PersonCurrentRolesSource < GraphQL::Dataloader::Source
def fetch(person_content_ids)
# rubocop:disable Lint/MissingSuper
def initialize
@content_store = :live
end
# rubocop:enable Lint/MissingSuper

def fetch(person_content_ids_and_selections)
person_content_ids = []
ids_map = {}
all_selections = {
role_appointment_links: %i[target_content_id],
documents: %i[content_id],
}
editions_selections = Set.new

person_content_ids_and_selections.each do |person_content_id, selections|
person_content_ids << person_content_id
ids_map[person_content_id] = []
editions_selections.merge(selections)
end

all_selections[:editions] = editions_selections.to_a

all_roles = Edition
.live
.includes(
document: {
reverse_links: { # role -> role_appointment
link_set: {
documents: [
:editions, # role_appointment
:link_set_links, # role_appointment -> person
],
},
},
},
.joins(document: [reverse_links: :link_set])
.joins(
<<~SQL,
INNER JOIN documents role_appointment_documents
ON role_appointment_documents.content_id = link_sets.content_id
SQL
)
.joins(
<<~SQL,
INNER JOIN editions role_appointment_editions
ON role_appointment_editions.document_id = role_appointment_documents.id
SQL
)
.joins(
<<~SQL,
INNER JOIN link_sets role_appointment_link_sets
ON role_appointment_link_sets.content_id = role_appointment_documents.content_id
SQL
)
.joins(
<<~SQL,
INNER JOIN links role_appointment_links
ON role_appointment_links.link_set_id = role_appointment_link_sets.id
SQL
)
.where(
document_type: "ministerial_role",
document: { locale: "en" },
editions: {
content_store: @content_store,
document_type: "ministerial_role",
},
documents: { locale: "en" },
reverse_links: { link_type: "role" },
documents: { locale: "en" }, # role_appointment Documents
editions_documents: { document_type: "role_appointment" }, # role_appointment Editions
link_set_links: { target_content_id: person_content_ids, link_type: "person" },
role_appointment_documents: { locale: "en" },
role_appointment_editions: {
content_store: @content_store,
document_type: "role_appointment",
},
role_appointment_links: {
target_content_id: person_content_ids,
link_type: "person",
},
)
.where("editions_documents.details ->> 'current' = 'true'") # editions_documents is the alias that Active Record gives to the role_appointment Editions in the SQL query
.where("role_appointment_editions.details ->> 'current' = 'true'")
.order(reverse_links: { position: :asc })

ids_map = person_content_ids.index_with { [] }
.select(all_selections)

all_roles.each_with_object(ids_map) { |role, hash|
hash[person_content_id_for_role(role)] << role
hash[role.target_content_id] << role
}.values
end

private

def person_content_id_for_role(role)
role_appointment_documents_for_role(role)
.flat_map(&:link_set_links)
.find { |link| link.link_type == "person" } # role_appointment -> person
.target_content_id
end

def role_appointment_documents_for_role(role)
role.document.reverse_links
.select { |link| link.link_type == "role" } # role -> role_appointment
.flat_map { |link| link.link_set.documents }
end
end
end
11 changes: 8 additions & 3 deletions app/graphql/types/ministers_index_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,16 @@ class MinistersIndexPersonDetails < Types::BaseObject
end

class MinistersIndexPersonLinks < Types::BaseObject
field :role_appointments, [MinistersIndexRoleAppointment]
field :role_appointments, [MinistersIndexRoleAppointment], extras: [:lookahead]

def role_appointments(lookahead:)
links_lookahead = lookahead.selections.find { _1.name == :links }
role_lookahead = links_lookahead.selections.find { _1.name == :role }

selections = convert_edition_selections(lookahead: role_lookahead)

def role_appointments
dataloader.with(Sources::PersonCurrentRolesSource)
.load(object.content_id)
.load([object.content_id, selections])
end
end

Expand Down
5 changes: 4 additions & 1 deletion spec/graphql/sources/person_current_roles_source_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@
appoint_person_to_role(person_1, role_3)

GraphQL::Dataloader.with_dataloading do |dataloader|
request = dataloader.with(described_class).request(person_1.content_id)
request = dataloader.with(described_class).request([
person_1.content_id,
%i[id base_path title details],
])

expect(request.load).to eq([role_1, role_3])
end
Expand Down

0 comments on commit 4dfce35

Please sign in to comment.