Skip to content

Commit

Permalink
Merge pull request #3629 from alphagov/revert-3627-derive-metadata-la…
Browse files Browse the repository at this point in the history 
…bels-from-allowed-facet-values-where-present

Revert "Derive search result metadata labels from allowed values on facet where present"
  • Loading branch information
@ryanb-gds
ryanb-gds authored Jan 16, 2025
2 parents 50b7e44 + ed838ea commit ef248a1
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 145 deletions.
64 changes: 28 additions & 36 deletions app/models/document.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ def date_metadata_keys(facets)
metadata_facets(facets).select { |f| f.type == "date" }.map(&:key)
end

def text_metadata_facets(facets)
facets = metadata_facets(facets).select { |f| f.type == "text" }
facets.reject do |facet|
facet.key == "organisations" && is_mainstream_content?
def text_metadata_keys(facets)
keys = metadata_facets(facets).select { |f| f.type == "text" }.map(&:key)
keys.reject do |key|
key == "organisations" && is_mainstream_content?
end
end

Expand Down Expand Up @@ -100,44 +100,36 @@ def build_date_metadata(key)
end

def text_metadata(facets)
metadata_labels = {}
text_metadata_facets(facets).each do |facet|
document_values = document_hash[facet.key]
next if document_values.nil?

metadata_labels[facet.key] = if facet.allowed_values.empty?
metadata_labels_from_document_values(document_values, facet.key)
else
metadata_labels_from_allowed_values(facet.allowed_values, document_values, facet.key)
end
end
metadata_labels.map(&method(:build_text_metadata)).select(&method(:metadata_value_present?))
text_metadata_keys(facets)
.map(&method(:build_text_metadata))
.select(&method(:metadata_value_present?))
end

def build_text_metadata(facet_key, values)
{
id: facet_key,
name: facet_key,
value: format_value(values),
labels: values,
type: "text",
}
end
def text_labels_for(key)
labels = Array(document_hash.fetch(key, []))
.map { |label| get_metadata_label(key, label) }
.select(&:present?)

def metadata_labels_from_document_values(document_values, facet_key)
if document_values.is_a?(Array)
document_values.map { |document_value| get_metadata_label(facet_key, document_value) }
else
[get_metadata_label(facet_key, document_values)]
if key == "organisations"
labels = labels.sort_by do |label|
label.sub("Closed organisation: ", "ZZ").upcase
end
end

labels
end

def metadata_labels_from_allowed_values(allowed_values, document_values, facet_key)
document_values.map do |document_value|
document_value_key = document_value.is_a?(Hash) ? document_value["value"] : document_value
matched_value = allowed_values.find { |allowed_value| allowed_value["value"] == document_value_key }
matched_value ? matched_value["label"] : get_metadata_label(facet_key, document_value)
end
def build_text_metadata(key)
labels = text_labels_for(key)
value = format_value(labels)

{
id: key,
name: key,
value:,
labels:,
type: "text",
}
end

def format_value(labels)
Expand Down
113 changes: 4 additions & 109 deletions spec/models/document_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,27 +65,7 @@
[FactoryBot.build(:option_select_facet, key: "a_filter_key")]
end

describe "and the document is not tagged to any values of the facet filter key" do
let(:document_hash) do
FactoryBot.build(:document_hash, a_filter_key: nil)
end

it "does not return any metadata" do
expect(described_class.new(document_hash, 1).metadata(facets)).to eq([])
end
end

describe "and the document values do not match the expected format" do
let(:document_hash) do
FactoryBot.build(:document_hash, a_filter_key: [{ slug: "some-url" }])
end

it "does not return any metadata" do
expect(described_class.new(document_hash, 1).metadata(facets)).to eq([])
end
end

describe "and the document is tagged to a single value of the facet filter key" do
describe "The document is tagged to a single value of the facet filter key" do
let(:document_hash) do
FactoryBot.build(:document_hash, a_filter_key: "metadata_label")
end
Expand All @@ -102,18 +82,18 @@
expect(described_class.new(document_hash, 1).metadata(facets)).to eq([expected_hash])
end

describe "and there is a short name in the facet" do
describe "There is a short name in the facet" do
let(:facets) do
[FactoryBot.build(:option_select_facet, short_name: "short name")]
end

it "replaces the name field in the metadata by the short name from the facet" do
it "replaces the name field in the metafata by the short name from the facet" do
expect(described_class.new(document_hash, 1).metadata(facets)).to match([include(name: "short name")])
end
end
end

describe "and the document is tagged to multiple values of the facet filter key" do
describe "The document is tagged to a multiple values of the facet filter key" do
let(:document_hash) do
FactoryBot.build(
:document_hash,
Expand All @@ -138,91 +118,6 @@
expect(described_class.new(document_hash, 1).metadata(facets)).to eq([expected_hash])
end
end

context "and the facet has a set of allowed values" do
let(:allowed_values) do
[
{ "label" => "metadata label 1", value: "metadata_label_1" },
{ "label" => "metadata label 2", value: "metadata_label_2" },
{ "label" => "metadata label 3", value: "metadata_label_3" },
]
end
let(:facets) do
[FactoryBot.build(:option_select_facet, key: "a_filter_key", allowed_values:)]
end

describe "and the document is tagged to multiple values of the facet filter key" do
let(:document_hash) do
FactoryBot.build(
:document_hash,
a_filter_key:
[
{ "label" => "metadata label 1", value: "metadata_label_1" },
{ "label" => "metadata label 3", value: "metadata_label_3" },
],
)
end

it "gets the metadata" do
expected_hash =
{
id: "a_filter_key",
name: "A filter key",
value: "metadata label 1 and 1 others",
labels: ["metadata label 1", "metadata label 3"],
type: "text",
}
expect(described_class.new(document_hash, 1).metadata(facets)).to eq([expected_hash])
end
end

describe "and the document is tagged to a multiple values of the facet filter key that do not match any allowed values" do
let(:document_hash) do
FactoryBot.build(
:document_hash,
a_filter_key:
[
{ "label" => "mismatched label 1", value: "mismatched_label_1" },
{ "label" => "mismatched label 3", value: "mismatched_label_3" },
],
)
end

it "gets the metadata" do
expected_hash =
{
id: "a_filter_key",
name: "A filter key",
value: "mismatched label 1 and 1 others",
labels: ["mismatched label 1", "mismatched label 3"],
type: "text",
}
expect(described_class.new(document_hash, 1).metadata(facets)).to eq([expected_hash])
end
end

describe "and the document is tagged to a multiple values of the facet filter key without search result expansion" do
let(:document_hash) do
FactoryBot.build(
:document_hash,
a_filter_key:
%w[metadata_label_1 metadata_label_3],
)
end

it "gets the metadata" do
expected_hash =
{
id: "a_filter_key",
name: "A filter key",
value: "metadata label 1 and 1 others",
labels: ["metadata label 1", "metadata label 3"],
type: "text",
}
expect(described_class.new(document_hash, 1).metadata(facets)).to eq([expected_hash])
end
end
end
end

describe "The facet key is an organisation or a document collection" do
Expand Down

0 comments on commit ef248a1

Please sign in to comment.