Skip to content

Commit

Permalink
Fix bug when filtering by org and all locales
Browse files Browse the repository at this point in the history
Previously, if the query included `locales: all` and contained
link_filters then we'd receive an SQL error:

```
PG::UndefinedTable: ERROR:  missing FROM-clause entry for table "documents"
LINE 1: ...s" INNER JOIN link_sets ON link_sets.content_id = documents....
```

This only occurred when using the `all` value for a locale due to the
`documents` join being skipped [1] and so then the link_sets join
failed [2] due to missing the `documents` table declaration.

To mitigate this, we've added the document join to the default scope as
it is unlikely we'd query without a document.

[1]: https://github.com/alphagov/publishing-api/blob/ff491b745bf7cfdd3ff8fba6dabbdabbe40c9150/app/queries/get_content_collection.rb#L52
[2]: https://github.com/alphagov/publishing-api/blob/ff491b745bf7cfdd3ff8fba6dabbdabbe40c9150/app/models/link.rb#L13
  • Loading branch information
1pretz1 committed Jun 16, 2023
1 parent ff491b7 commit fc4a569
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
4 changes: 2 additions & 2 deletions app/queries/get_content_collection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ def call
)

def editions
scope = Edition.all
scope = Edition.with_document
scope = scope.where(document_type: document_types) if document_types.any?
scope = scope.where(publishing_app:) if publishing_app
scope = scope.with_document.where("documents.locale": locale) unless locale == "all"
scope = scope.where("documents.locale": locale) unless locale == "all"
scope = Link.filter_editions(scope, link_filters) if link_filters.present?
scope
end
Expand Down
16 changes: 15 additions & 1 deletion spec/queries/get_content_collection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,17 @@
document_type: "mainstream_browse_page",
schema_name: "mainstream_browse_page",
)
create(
draft_d = create(
:draft_edition,
base_path: "/d",
document_type: "another_type",
schema_name: "another_type",
)
create(
:link_set,
content_id: draft_d.content_id,
links_hash: { organisations: %w[af07d5a5-df63-4ddc-9383-6a666845ebe9] },
)
end

it "returns the requested fields for all editions" do
Expand All @@ -38,6 +43,15 @@
])
end

it "returns editions for an organisation for all locales" do
expect(Queries::GetContentCollection.new(
fields: %w[base_path],
filters: { locale: "all", links: { organisations: "af07d5a5-df63-4ddc-9383-6a666845ebe9" } },
).call).to match_array([
hash_including("base_path" => "/d"),
])
end

it "returns the editions matching the type" do
expect(Queries::GetContentCollection.new(
document_types: "topic",
Expand Down

0 comments on commit fc4a569

Please sign in to comment.