Skip to content

Commit

Permalink
Merge pull request #3630 from alphagov/reinstate-metadata-labels-from…
Browse files Browse the repository at this point in the history 
…-allowed-values

Reinstate metadata labels from allowed values
  • Loading branch information
@ryanb-gds
ryanb-gds authored Jan 17, 2025
2 parents a8c12c5 + 5304320 commit 1707cbd
Show file tree
Hide file tree
Showing 2 changed files with 174 additions and 35 deletions.
65 changes: 37 additions & 28 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_keys(facets)
keys = metadata_facets(facets).select { |f| f.type == "text" }.map(&:key)
keys.reject do |key|
key == "organisations" && is_mainstream_content?
def text_metadata_facets(facets)
facets = metadata_facets(facets).select { |f| f.type == "text" }
facets.reject do |facet|
facet.key == "organisations" && is_mainstream_content?
end
end

Expand Down Expand Up @@ -100,38 +100,47 @@ def build_date_metadata(key)
end

def text_metadata(facets)
text_metadata_keys(facets)
.map(&method(:build_text_metadata))
.select(&method(:metadata_value_present?))
end

def text_labels_for(key)
labels = Array(document_hash.fetch(key, []))
.map { |label| get_metadata_label(key, label) }
.select(&:present?)

if key == "organisations"
labels = labels.sort_by do |label|
label.sub("Closed organisation: ", "ZZ").upcase
end
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

labels
metadata_labels.map(&method(:build_text_metadata)).select(&method(:metadata_value_present?))
end

def build_text_metadata(key)
labels = text_labels_for(key)
value = format_value(labels)

def build_text_metadata(facet_key, values)
{
id: key,
name: key,
value:,
labels:,
id: facet_key,
name: facet_key,
value: format_value(values),
labels: values,
type: "text",
}
end

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)]
end
end

def metadata_labels_from_allowed_values(allowed_values, document_values, facet_key)
document_values = [document_values] unless document_values.is_a?(Array)
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
end

def format_value(labels)
if labels.count > 1
"#{labels.first} and #{labels.count - 1} others"
Expand Down
144 changes: 137 additions & 7 deletions spec/models/document_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,35 +65,80 @@
[FactoryBot.build(:option_select_facet, key: "a_filter_key")]
end

describe "The document is tagged to a single value of the facet filter key" do
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: "metadata_label")
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
let(:document_hash) do
FactoryBot.build(:document_hash, a_filter_key: "metadata_label_1")
end

it "gets metadata for a simple text value" do
expected_hash =
{
id: "a_filter_key",
name: "A filter key",
value: "metadata_label",
labels: %w[metadata_label],
value: "metadata_label_1",
labels: %w[metadata_label_1],
type: "text",
}
expect(described_class.new(document_hash, 1).metadata(facets)).to eq([expected_hash])
end

describe "There is a short name in the facet" do
describe "and 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 metafata by the short name from the facet" do
it "replaces the name field in the metadata 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

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

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

describe "The document is tagged to a multiple values of the facet filter key" do
describe "and the document is tagged to multiple values of the facet filter key" do
let(:document_hash) do
FactoryBot.build(
:document_hash,
Expand All @@ -118,6 +163,91 @@
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 1707cbd

Please sign in to comment.