Skip to content

Commit

Permalink
Merge pull request #2847 from alphagov/revert-2823-content-modelling/…
Browse files Browse the repository at this point in the history
…377-improve-link-handling-in-content-endpoint

Revert "Improve link handling in content endpoint"
  • Loading branch information
tahb committed Aug 21, 2024
2 parents 6f64fdc + 213e23f commit 6cee925
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 121 deletions.
7 changes: 2 additions & 5 deletions app/models/link.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,10 @@ def self.filter_editions(scope, filters)
logger.warn("filter_editions called with multiple filters. These will be ANDed together in a way that probably isn't what we want. Filters were: #{filters.inspect}")
end

scope = scope.joins("LEFT JOIN links edition_links ON edition_links.edition_id = editions.id")
scope = scope.left_joins(document: :link_set_links)
scope = scope.joins(document: :link_set_links)

filters.each do |link_type, target_content_id|
scope = scope
.where(edition_links: { link_type:, target_content_id: })
.or(scope.where(links: { link_type:, target_content_id: }))
scope = scope.where(links: { link_type:, target_content_id: })
end

scope
Expand Down
16 changes: 1 addition & 15 deletions app/presenters/queries/content_item_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,6 @@ def field_selector(field)
nil
when :links
nil
when :link_set_links
nil
when :total
nil
else
Expand Down Expand Up @@ -182,24 +180,13 @@ def select_fields(scope)
(
SELECT json_agg((links.link_type, links.target_content_id)) AS links
FROM links
LEFT JOIN link_sets on links.link_set_id = link_sets.id
WHERE links.edition_id = subquery.id
) links_subquery
SQL

LINK_SET_LINKS_SQL = <<-SQL.freeze
(
SELECT json_agg((links.link_type, links.target_content_id)) AS link_set_links
FROM links
LEFT JOIN link_sets on links.link_set_id = link_sets.id
WHERE link_sets.content_id = content_id
) link_set_links_subquery
SQL

LATERAL_AGGREGATES = {
state_history: STATE_HISTORY_SQL,
links: LINKS_SQL,
link_set_links: LINK_SET_LINKS_SQL,
}.freeze

def join_lateral_aggregates(scope)
Expand Down Expand Up @@ -243,7 +230,7 @@ def join_lateral_aggregates(scope)
SQL

def parse_results(results)
json_columns = %w[details routes redirects state_history unpublishing links link_set_links]
json_columns = %w[details routes redirects state_history unpublishing links]
int_columns = %w[user_facing_version lock_version]

Enumerator.new do |yielder|
Expand All @@ -254,7 +241,6 @@ def parse_results(results)

parse_state_history(result)
parse_links(result, "links")
parse_links(result, "link_set_links")

result.slice!(*fields.map(&:to_s))

Expand Down
2 changes: 1 addition & 1 deletion app/queries/get_content_collection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def validate_fields!
end

def permitted_fields
default_fields + %w[total link_set_links]
default_fields + %w[total]
end

def default_fields
Expand Down
36 changes: 8 additions & 28 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -366,34 +366,14 @@ and a state has been specified, the draft is returned.
- `document_type` *(optional)*
- The type of editions to return.
- `fields[]` *(optional)*
- Accepts an array of:
- analytics_identifier
- base_path
- content_store
- description
- details
- document_type
- first_published_at
- last_edited_at
- links
- link_set_links
- major_published_at
- phase
- public_updated_at
- published_at
- publishing_api_first_published_at
- publishing_api_last_edited_at
- publishing_app
- redirects
- rendering_app
- routes
- schema_name
- state
- title
- user_facing_version
- update_type
- Determines which fields will be returned in the response, if omitted all fields (with the exception of
`link_set_links`) will be returned.
- Accepts an array of: analytics_identifier, base_path,
content_store, description, details, document_type,
first_published_at, last_edited_at, major_published_at, phase,
public_updated_at, published_at, publishing_api_first_published_at,
publishing_api_last_edited_at, publishing_app, redirects, rendering_app,
routes, schema_name, state, title, user_facing_version, update_type
- Determines which fields will be returned in the response, if omitted all
fields will be returned.
- `link_*` *(optional)*
- Accepts a content_id.
- Used to restrict documents to those linking to another document,
Expand Down
9 changes: 3 additions & 6 deletions spec/models/link_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,9 @@
let(:scope) { double(:scope) }

it "modifies a scope to filter linked editions" do
expect(scope).to receive(:joins).with("LEFT JOIN links edition_links ON edition_links.edition_id = editions.id").and_return(scope)
expect(scope).to receive(:left_joins).with(document: :link_set_links).and_return(scope)

expect(scope).to receive(:where).with(edition_links: { link_type: "organisations", target_content_id: "12345" }).and_return(scope)
expect(scope).to receive(:where).with(links: { link_type: "organisations", target_content_id: "12345" }).and_return(scope)
expect(scope).to receive(:or).with(scope)
expect(scope).to receive(:joins).with(anything).and_return(scope)
expect(scope).to receive(:where)
.with(links: { link_type: "organisations", target_content_id: "12345" })

described_class.filter_editions(scope, "organisations" => "12345")
end
Expand Down
40 changes: 0 additions & 40 deletions spec/presenters/queries/content_item_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,46 +164,6 @@
expect(result).to eq expected
end
end

context "with linkset links" do
let(:result) { described_class.present(edition, fields: %i[link_set_links]) }

context "when we have a linkset link" do
before do
link_set = create(:link_set, content_id: edition.document.content_id)
link_set.links.create!(link_type: "test", target_content_id: content_id)
end

it "presents the item including the link" do
expected = {
"link_set_links" => { "test" => [content_id] },
}
expect(result).to eq expected
end
end

context "when we have multiple linkset links" do
let(:other_content_id) { SecureRandom.uuid }
let(:and_another_content_id) { SecureRandom.uuid }

before do
link_set = create(:link_set, content_id: edition.document.content_id)
link_set.links.create!(link_type: "test", target_content_id: other_content_id)
link_set.links.create!(link_type: "test", target_content_id: and_another_content_id)
link_set.links.create!(link_type: "ers", target_content_id: content_id)
end

it "presents the item including the link" do
expected = {
"link_set_links" => {
"test" => [other_content_id, and_another_content_id],
"ers" => [content_id],
},
}
expect(result).to eq expected
end
end
end
end

describe "#present_many" do
Expand Down
26 changes: 0 additions & 26 deletions spec/queries/get_content_collection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -278,16 +278,13 @@

describe "filtering by links" do
let(:someorg_content_id) { SecureRandom.uuid }
let(:other_link_content_id) { SecureRandom.uuid }

before do
otherorg_content_id = SecureRandom.uuid
draft_1_content_id = SecureRandom.uuid
draft_2_content_id = SecureRandom.uuid
live_1_content_id = SecureRandom.uuid

edition_linked_content_id = SecureRandom.uuid

create(
:draft_edition,
document: create(:document, content_id: draft_1_content_id),
Expand All @@ -307,25 +304,13 @@
base_path: "/baz",
)

edition_linked_edition = create(
:live_edition,
document: create(:document, content_id: edition_linked_content_id),
base_path: "/boo",
)

link_set1 = create(:link_set, content_id: draft_1_content_id)
link_set2 = create(:link_set, content_id: draft_2_content_id)
link_set3 = create(:link_set, content_id: live_1_content_id)

create(:link, link_set: link_set1, target_content_id: someorg_content_id)
create(:link, link_set: link_set2, target_content_id: otherorg_content_id)
create(:link, link_set: link_set3, target_content_id: someorg_content_id)

create(:link,
target_content_id: other_link_content_id,
edition_id: edition_linked_edition.id,
link_type: "something_else",
link_set: nil)
end

it "filters editions by organisation" do
Expand All @@ -340,17 +325,6 @@
])
end

it "filters editions by an edition link" do
result = Queries::GetContentCollection.new(
filters: { links: { something_else: other_link_content_id } },
fields: %w[base_path],
).call

expect(result).to match_array([
hash_including("base_path" => "/boo"),
])
end

it "filters editions by organisation and other filters" do
result = Queries::GetContentCollection.new(
filters: {
Expand Down

0 comments on commit 6cee925

Please sign in to comment.