Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent government_frontend test failure if meta tag key doesn't exist #3741

Merged
merged 1 commit into from
Dec 4, 2023
Merged

Prevent government_frontend test failure if meta tag key doesn't exist #3741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
useful summary for people upgrading their application, not a replication
of the commit log.

## Unreleased

* Prevent government_frontend test failure if meta tag key doesn't exist ([PR #3741](https://github.com/alphagov/govuk_publishing_components/pull/3741))

## 36.0.1

* Use component wrapper on option select ([PR #3738](https://github.com/alphagov/govuk_publishing_components/pull/3738))
Expand Down
11 changes: 5 additions & 6 deletions lib/govuk_publishing_components/presenters/meta_tags.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,12 @@ def add_political_tags(meta_tags)
def add_ga4_political_tags(meta_tags)
government = content_item.dig(:links, :government, 0)

# political: true/false is in a different place to current: true/false, which is why we have 'details' and 'government[:details]'
# political: true/false is in a different place to current: true/false, which is why we have 'details' and 'government.dig(:details)'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment still relevant? I'm not sure what it means.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andysellick Populating the meta tag relies on two values existing:

details[:political]: (true/false)
government[:details][:current]: (true/false)

And the comment was trying to explain why the code is referring to two different details objects.

if government && details[:political]
meta_tags["govuk:ga4-political-status"] = government[:details][:current] ? "political" : "historic"

government_title = government[:title]
if government_title && !government[:details][:current]
meta_tags["govuk:ga4-publishing-government"] = government_title
current_government = government.dig(:details, :current)
unless current_government
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm struggling to get my head round this a bit so bear with me. This seems like quite a significant change? It used to always set the ga4-political-status meta tag (either to political or historic) but now it seems like it might not set it at all, if there's a current_government. And it can only ever be historic, not political?

Copy link
Contributor Author

@AshGDS AshGDS Dec 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andysellick The PAs wanted this meta tag to show on "This was published by X government" pages, and that only appears if the page was not created during the current government. If the page was written during the current government you get the "political" value, but if it was published by a previous government, it gives "historic". I had a test which was meant to check the meta tag doesn't render for the current government previously, but it was passing due to the false positive.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, sorry, just a bit confused. I can see that ga4-political-status gets set to historic on line 99, but where does it get set to political? Does that come directly from the content somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andysellick No worries - It no longer gets set to political at all with this change since that's only relevant for pages that are published during the current government. Because the PAs have only requested to have the information of previous governments I have removed it from being set.

Political would get set if current is true for the government in the content item. In the old meta tag, it was setting the meta tag to political based on that:

if details[:political]
  political_status = details[:government][:current] ? "political" : "historic"
  end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay that's clarified things. Thanks.

meta_tags["govuk:ga4-political-status"] = "historic"
meta_tags["govuk:ga4-publishing-government"] = government[:title] if government[:title]
end
end

Expand Down
55 changes: 33 additions & 22 deletions spec/components/meta_tags_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,10 +440,10 @@ def example_document_for(schema_name, example_name)
end

it "doesn't render govuk:ga4-browse-topic if the dig doesn't return anything" do
content_item = {
"links": nil,
}
render_component(content_item: example_document_for("transaction", "transaction").merge(content_item))
content_item = example_document_for("html_publication", "published_with_history_mode")
content_item.delete("links")

render_component(content_item:)
assert_no_meta_tag("govuk:ga4-browse-topic")
end

Expand All @@ -454,46 +454,57 @@ def example_document_for(schema_name, example_name)
end

it "doesn't render GA4 political tags if the government object doesn't exist in the content item" do
content_item = {
"links": {
"government": nil,
},
}
render_component(content_item: example_document_for("html_publication", "published_with_history_mode").merge(content_item))
content_item = example_document_for("html_publication", "published_with_history_mode")
content_item["links"].delete("government")

render_component(content_item:)
assert_no_meta_tag("govuk:ga4-publishing-government")
assert_no_meta_tag("govuk:ga4-political-status")
end

it "doesn't crash generating GA4 political tags if details/current does not exist" do
content_item = example_document_for("html_publication", "published_with_history_mode")
content_item["links"]["government"][0].delete("details")

render_component(content_item:)
assert_meta_tag("govuk:ga4-publishing-government", "2010 to 2015 Conservative and Liberal Democrat coalition government")
assert_meta_tag("govuk:ga4-political-status", "historic")
end

it "doesn't render GA4 political tags if the government object exists in the content item, but political is false" do
content_item = {
"political": false,
"links": {
"government": {
content_item = example_document_for("html_publication", "published_with_history_mode")
content_item["details"]["political"] = false
content_item["links"] = {
"government": [
{
"details": {
"current": false,
},
"title": "2005 to 2010 Labour government",
},
},
],
}
render_component(content_item: example_document_for("html_publication", "published_with_history_mode").merge(content_item))

render_component(content_item:)
assert_no_meta_tag("govuk:ga4-publishing-government")
assert_no_meta_tag("govuk:ga4-political-status")
end

it "doesn't render GA4 political tags if the government object exists in the content item, but it refers to the current government" do
content_item = {
"political": true,
"links": {
"government": {
content_item = example_document_for("html_publication", "published_with_history_mode")
content_item["details"]["political"] = true
content_item["links"] = {
"government": [
{
"details": {
"current": true,
},
"title": "2015 Conservative government",
},
},
],
}
render_component(content_item: example_document_for("html_publication", "published_with_history_mode").merge(content_item))

render_component(content_item:)
assert_no_meta_tag("govuk:ga4-publishing-government")
assert_no_meta_tag("govuk:ga4-political-status")
end
Expand Down