Skip to content

Commit

Permalink
SELECT subset of columns for role_appointments
Browse files Browse the repository at this point in the history
This "optimisation" is specific to the ministers index type.

We've followed the patterns established in the root Edition and the more
generic Link sources. The big differences are mostly down to
* the larger number of JOINs used in this query
* the convenience of only needing to handle link set links here
* this data loader fetches a person's roles but is actually invoked from
  the person's role_appointments field... weird

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 authored and yndajas committed Mar 4, 2025
1 parent 27b6c4e commit 0359c2f
Show file tree
Hide file tree
Showing 3 changed files with 81 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
16 changes: 13 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,21 @@ 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)

if role_lookahead.selects?(:links)
selections << :id
selections << :content_store
end

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 0359c2f

Please sign in to comment.