From 200b2c78ffa39c216530c6a0f902ee75a2093ddf Mon Sep 17 00:00:00 2001
From: David Singleton <david.singleton@digital.cabinet-office.gov.uk>
Date: Fri, 15 Apr 2016 10:20:53 +0100
Subject: [PATCH 1/2] Add register-to-vote promo on transaction done pages

Only on completed transaction format (though we'll extend it to transaction
pages in the near future). Exclude register to vote done page, as there is
not a lot of point promoing something you've just done.

As some done pages will have register-to-vote links in their related links
we need to exclude those, and if they're the only link in a group, the
group as well.

Filtering the register-to-vote link out of the related_artefacts before
grouping makes this much easier, but should only be done for pages where
we're showing the promo.

Note: register-to-vote is filtered out by content id, as the related
artefacts don't have path/slug field, and the full web_url is pretty
long, and can vary per environment, so use the content_id, which should
be stable.

Note: the changes to the tests are not super elegant, but this change is
likely to be reverted wholesale once the promo is complete, this isn't a
pattern we're planning to keep around.
---
 app/views/root/related.raw.html.erb       | 29 +++++++++++++++++++++--
 test/integration/related_template_test.rb | 25 +++++++++++++++----
 2 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/app/views/root/related.raw.html.erb b/app/views/root/related.raw.html.erb
index 41e76551c..6f1984160 100644
--- a/app/views/root/related.raw.html.erb
+++ b/app/views/root/related.raw.html.erb
@@ -1,10 +1,34 @@
 <%# Whilst they are both in use, this and the test copy in slimmer should be kept in sync %>
-<% if artefact and (artefact.related_artefacts.any? or (artefact.related_external_links and artefact.related_external_links.any?)) %>
+
+<%
+  promo_target_formats = ['completed_transaction', 'transaction']
+  promo_excluded_regex = /register-to-vote/
+
+  has_promo_callout = (artefact and promo_target_formats.include?(artefact.format) and not promo_excluded_regex.match(URI.decode(artefact.slug)))
+%>
+<%# Whilst they are both in use, this and the test copy in slimmer should be kept in sync %>
+<% if artefact and (artefact.related_artefacts.any? or (artefact.related_external_links and artefact.related_external_links.any?) or has_promo_callout) %>
   <!-- related -->
   <div class="related-container">
 
     <aside class="related" id="related">
-      <% artefact.related_artefacts.group_by(&:group).each do |group, related_artefacts| %>
+
+      <% if has_promo_callout %>
+        <div class="inner group related-callout">
+          <h2 id="parent-promo">Register to vote</h2>
+          <p>You need to register if you want to vote in elections and referendums.</p>
+          <p>You can <a href="/register-to-vote?source=related">register online</a> in less than 5 minutes.</p>
+        </div>
+      <% end %>
+
+      <% related_artefacts = artefact.related_artefacts || [] %>
+
+      <%# Exclude '/register-to-vote', only if we're adding it manually (its easier to do by `content_id` than full `web_url`) %>
+      <% related_artefacts = related_artefacts.reject { |artefact|
+        artefact.content_id == "834a7921-260b-4061-9de1-edda3e998c68" and has_promo_callout
+      } %>
+
+      <% related_artefacts.group_by(&:group).each do |group, related_artefacts| %>
         <%
           section = case group
                     when 'subsection' then artefact.primary_section
@@ -12,6 +36,7 @@
                     when 'other' then { "title" => "Elsewhere on GOV.UK" }
                     end
           section = section['parent'] if artefact.format == "travel-advice" and group == "subsection"
+
         %>
         <div class="inner group related-<%= group %>">
           <% if section %>
diff --git a/test/integration/related_template_test.rb b/test/integration/related_template_test.rb
index 66949315c..0e07e74eb 100644
--- a/test/integration/related_template_test.rb
+++ b/test/integration/related_template_test.rb
@@ -10,9 +10,9 @@ def get_template
 
   context "with related artefacts" do
     setup do
-      @related1 = stub("Artefact", :web_url => "http://www.example.com/foo", :title => "Foo", :group => "subsection")
-      @related2 = stub("Artefact", :web_url => "http://www.example.com/bar", :title => "Bar", :group => "section")
-      @related3 = stub("Artefact", :web_url => "http://www.example.com/baz", :title => "Baz", :group => "other")
+      @related1 = stub("Artefact", :web_url => "http://www.example.com/foo", :title => "Foo", :group => "subsection", :content_id => 1)
+      @related2 = stub("Artefact", :web_url => "http://www.example.com/bar", :title => "Bar", :group => "section", :content_id => 2)
+      @related3 = stub("Artefact", :web_url => "http://www.example.com/baz", :title => "Baz", :group => "other", :content_id => 3)
       @artefact = stub("Artefact",
         :related_artefacts => [@related1, @related2, @related3],
         :related_external_links => [],
@@ -207,7 +207,7 @@ def get_template
   should "be blank with an artefact with no related_artefacts or related_external_links" do
     template = get_template
 
-    artefact = stub("Artefact", :related_artefacts => [], :related_external_links => [])
+    artefact = stub("Artefact", :related_artefacts => [], :related_external_links => [], :related_external_links => nil, :format => "smart_answer")
     result = ERB.new(template).result(binding)
 
     assert_match /\A\s+\z/, result
@@ -216,9 +216,24 @@ def get_template
   should "be blank with no related_artefacts and nil related_external_links" do
     template = get_template
 
-    artefact = stub("Artefact", :related_artefacts => [], :related_external_links => nil)
+    artefact = stub("Artefact", :related_artefacts => [], :related_external_links => nil, :format => "smart_answer")
     result = ERB.new(template).result(binding)
 
     assert_match /\A\s+\z/, result
   end
+
+  context "adding promo block to related links" do
+    should "have promo block for target formats" do
+      artefact = stub("Artefact", :related_artefacts => [], :related_external_links => nil, :format => "completed_transaction", :slug => "book-theory-test")
+      result = ERB.new(get_template).result(binding)
+      doc = Nokogiri::HTML.parse(result)
+      assert doc.at_css("h2#parent-promo")
+    end
+
+    should "not have promo block for artefact with excluded path" do
+      artefact = stub("Artefact", :related_artefacts => [], :related_external_links => nil, :format => "completed_transaction", :slug => "done/register-to-vote")
+      result = ERB.new(get_template).result(binding)
+      assert_match /\A\s+\z/, result
+    end
+  end
 end

From 4eb245f3929b191a268d6f41e5d7fa3244141355 Mon Sep 17 00:00:00 2001
From: David Singleton <david.singleton@digital.cabinet-office.gov.uk>
Date: Tue, 19 Apr 2016 13:03:13 +0100
Subject: [PATCH 2/2] Fix linting errors in related links tests

The test code is pretty old, and far from our ruby style guide. We're doing diff
based linting, so this hasn't come up until i made changes to the tests
---
 test/integration/related_template_test.rb | 36 +++++++++++------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/test/integration/related_template_test.rb b/test/integration/related_template_test.rb
index 0e07e74eb..68604660c 100644
--- a/test/integration/related_template_test.rb
+++ b/test/integration/related_template_test.rb
@@ -10,15 +10,15 @@ def get_template
 
   context "with related artefacts" do
     setup do
-      @related1 = stub("Artefact", :web_url => "http://www.example.com/foo", :title => "Foo", :group => "subsection", :content_id => 1)
-      @related2 = stub("Artefact", :web_url => "http://www.example.com/bar", :title => "Bar", :group => "section", :content_id => 2)
-      @related3 = stub("Artefact", :web_url => "http://www.example.com/baz", :title => "Baz", :group => "other", :content_id => 3)
+      @related1 = stub("Artefact", web_url: "http://www.example.com/foo", title: "Foo", group: "subsection", content_id: 1)
+      @related2 = stub("Artefact", web_url: "http://www.example.com/bar", title: "Bar", group: "section", content_id: 2)
+      @related3 = stub("Artefact", web_url: "http://www.example.com/baz", title: "Baz", group: "other", content_id: 3)
       @artefact = stub("Artefact",
-        :related_artefacts => [@related1, @related2, @related3],
-        :related_external_links => [],
-        :primary_root_section => { "title" => "Section", "content_with_tag" => { "web_url" => "/browse/section" }},
-        :primary_section => { "title" => "Sub-section", "content_with_tag" => { "web_url" => "/browse/section/subsection" } },
-        :format => "guide"
+        related_artefacts: [@related1, @related2, @related3],
+        related_external_links: [],
+        primary_root_section: { "title" => "Section", "content_with_tag" => { "web_url" => "/browse/section" } },
+        primary_section: { "title" => "Sub-section", "content_with_tag" => { "web_url" => "/browse/section/subsection" } },
+        format: "guide"
       )
     end
 
@@ -148,11 +148,11 @@ def get_template
   context "adding related_external_links with no internal related links" do
     setup do
       @artefact = stub("Artefact",
-        :related_artefacts => [],
-        :related_external_links => [],
-        :primary_root_section => { "title" => "Section", "content_with_tag" => { "web_url" => "/browse/section" }},
-        :primary_section => { "title" => "Sub-section", "content_with_tag" => { "web_url" => "/browse/section/subsection" } },
-        :format => "guide"
+        related_artefacts: [],
+        related_external_links: [],
+        primary_root_section: { "title" => "Section", "content_with_tag" => { "web_url" => "/browse/section" } },
+        primary_section: { "title" => "Sub-section", "content_with_tag" => { "web_url" => "/browse/section/subsection" } },
+        format: "guide"
       )
     end
 
@@ -207,7 +207,7 @@ def get_template
   should "be blank with an artefact with no related_artefacts or related_external_links" do
     template = get_template
 
-    artefact = stub("Artefact", :related_artefacts => [], :related_external_links => [], :related_external_links => nil, :format => "smart_answer")
+    artefact = stub("Artefact", related_artefacts: [], related_external_links: [], format: "smart_answer")
     result = ERB.new(template).result(binding)
 
     assert_match /\A\s+\z/, result
@@ -216,7 +216,7 @@ def get_template
   should "be blank with no related_artefacts and nil related_external_links" do
     template = get_template
 
-    artefact = stub("Artefact", :related_artefacts => [], :related_external_links => nil, :format => "smart_answer")
+    artefact = stub("Artefact", related_artefacts: [], related_external_links: nil, format: "smart_answer")
     result = ERB.new(template).result(binding)
 
     assert_match /\A\s+\z/, result
@@ -224,16 +224,16 @@ def get_template
 
   context "adding promo block to related links" do
     should "have promo block for target formats" do
-      artefact = stub("Artefact", :related_artefacts => [], :related_external_links => nil, :format => "completed_transaction", :slug => "book-theory-test")
+      artefact = stub("Artefact", related_artefacts: [], related_external_links: nil, format: "completed_transaction", slug: "book-theory-test")
       result = ERB.new(get_template).result(binding)
       doc = Nokogiri::HTML.parse(result)
       assert doc.at_css("h2#parent-promo")
     end
 
     should "not have promo block for artefact with excluded path" do
-      artefact = stub("Artefact", :related_artefacts => [], :related_external_links => nil, :format => "completed_transaction", :slug => "done/register-to-vote")
+      artefact = stub("Artefact", related_artefacts: [], related_external_links: nil, format: "completed_transaction", slug: "done/register-to-vote")
       result = ERB.new(get_template).result(binding)
-      assert_match /\A\s+\z/, result
+      assert_match(/\A\s+\z/, result)
     end
   end
 end