From ad1d2b8ef3715f20d6b96253b43cb827928a578b Mon Sep 17 00:00:00 2001
From: Andrew Garner <andrew@andrewgarner.com>
Date: Mon, 16 Oct 2017 17:08:39 +0100
Subject: [PATCH 1/4] Use a Page Object for audit feature spec

This refactors the audit feature spec by introducing a semantic DSL for
describing the audit page elements using the Page Object Model pattern.

See:
  - https://docs.publishing.service.gov.uk/manual/testing.html
  - https://martinfowler.com/bliki/PageObject.html
  - https://robots.thoughtbot.com/better-acceptance-tests-with-page-objects
  - https://github.com/natritmeyer/site_prism
---
 Gemfile                                       |   1 +
 Gemfile.lock                                  |   6 +-
 spec/features/audit/audit/audit_spec.rb       | 227 +++++++++---------
 spec/rails_helper.rb                          |   3 +
 .../support/pages/audit/content_audit_tool.rb |   7 +
 .../audit_content_item_page.rb                |  31 +++
 6 files changed, 157 insertions(+), 118 deletions(-)
 create mode 100644 spec/support/pages/audit/content_audit_tool.rb
 create mode 100644 spec/support/pages/audit/content_audit_tool/audit_content_item_page.rb

diff --git a/Gemfile b/Gemfile
index 55d64fbc0..0add69eac 100644
--- a/Gemfile
+++ b/Gemfile
@@ -59,6 +59,7 @@ group :development, :test do
   gem 'rails-controller-testing'
   gem 'rspec-rails'
   gem 'simplecov', require: false
+  gem 'site_prism'
   gem 'spring'
   gem 'spring-commands-rspec'
   gem 'spring-watcher-listen'
diff --git a/Gemfile.lock b/Gemfile.lock
index fd01a1440..f4f48651c 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -417,6 +417,9 @@ GEM
       json (>= 1.8, < 3)
       simplecov-html (~> 0.10.0)
     simplecov-html (0.10.2)
+    site_prism (2.9)
+      addressable (>= 2.3.3, < 3.0)
+      capybara (>= 2.1, < 3.0)
     slop (3.6.0)
     spring (2.0.2)
       activesupport (>= 4.2)
@@ -519,6 +522,7 @@ DEPENDENCIES
   sass-rails
   selectize-rails
   simplecov
+  site_prism
   spring
   spring-commands-rspec
   spring-watcher-listen
@@ -534,4 +538,4 @@ RUBY VERSION
    ruby 2.3.1p112
 
 BUNDLED WITH
-   1.15.3
+   1.15.4
diff --git a/spec/features/audit/audit/audit_spec.rb b/spec/features/audit/audit/audit_spec.rb
index 1c0b01e65..1907fe092 100644
--- a/spec/features/audit/audit/audit_spec.rb
+++ b/spec/features/audit/audit/audit_spec.rb
@@ -24,145 +24,138 @@
     )
   end
 
-  def answer_question(question, answer)
-    find('p', text: question)
-      .first(:xpath, '..//..')
-      .choose(answer)
-  end
+  scenario "auditing a content item" do
+    audit_content_item = ContentAuditTool.new.audit_content_item
+    audit_content_item.load(content_id: content_item.content_id)
 
-  def expect_answer(question, answer)
-    label_element = find('p', text: question)
-                      .first(:xpath, "..//..//input[@type='radio'][@checked='checked']//..")
+    expect(page).to_not have_selector(".nav")
 
-    expect(label_element).to have_content(answer)
-  end
+    expect(audit_content_item.content_item_title)
+      .to have_link('Flooding', href: 'https://gov.uk/flooding')
 
-  scenario "auditing a content item" do
-    visit content_item_audit_path(content_item)
+    expect(audit_content_item)
+      .to have_content_item_description(text: 'All about flooding.')
 
-    expect(page).to_not have_selector(".nav")
+    expect(audit_content_item)
+      .to have_questions_title(text: 'Do these things need to change?')
 
-    expect(page).to have_link("Flooding", href: "https://gov.uk/flooding")
-    expect(page).to have_content("All about flooding.")
-
-    expect(page).to have_content("Do these things need to change?")
-
-    answer_question "Title", "No"
-    answer_question "Summary", "Yes"
-    answer_question "Page detail", "No"
-    fill_in "Notes", with: "something"
-
-    click_on "Save and continue"
-    expect(page).to have_content("Please answer all the questions.")
-
-    answer_question "Attachments", "Yes"
-    answer_question "Content type", "No"
-    answer_question "Is the content out of date?", "Yes"
-
-    answer_question "Should the content be removed?", "Yes"
-    expect(page).to have_content("Where should users be redirected to? (optional)")
-    fill_in "Where should users be redirected to? (optional)", with: "http://www.example.com"
-
-    answer_question "Is this content very similar to other pages?", "Yes"
-    fill_in "URLs of similar pages", with: "something"
-
-    click_on "Save and continue"
-    expect(page).to have_content("Audit saved — no items remaining.")
-
-    expect_answer "Title", "No"
-    expect_answer "Summary", "Yes"
-    expect_answer "Page detail", "No"
-    expect_answer "Attachments", "Yes"
-    expect_answer "Content type", "No"
-    expect_answer "Is the content out of date?", "Yes"
-    expect_answer "Should the content be removed?", "Yes"
-    expect(find_field("Where should users be redirected to? (optional)").value).to eq("http://www.example.com")
-    expect_answer "Is this content very similar to other pages?", "Yes"
-    expect(find_field("URLs of similar pages").value).to eq("something")
-    expect(find_field("Notes").value).to eq("something")
-
-    answer_question "Attachments", "Yes"
-    answer_question "Content type", "No"
-    answer_question "Is the content out of date?", "Yes"
-
-    click_on "Save and continue"
-    expect(page).to have_content("Audit saved — no items remaining.")
-
-    expect_answer "Title", "No"
-    expect_answer "Summary", "Yes"
-    expect_answer "Page detail", "No"
-    expect_answer "Attachments", "Yes"
-    expect_answer "Content type", "No"
-    expect_answer "Is the content out of date?", "Yes"
+    audit_content_item.audit_form do |form|
+      form.title.choose 'No'
+      form.summary.choose 'Yes'
+      form.page_detail.choose 'No'
+      form.notes.set 'something'
+      form.save_and_continue.click
+    end
+
+    expect(audit_content_item)
+      .to have_error_message(text: 'Please answer all the questions.')
+
+    audit_content_item.audit_form do |form|
+      form.attachments.choose 'Yes'
+      form.content_type.choose 'No'
+      form.content_out_of_date.choose 'Yes'
+      form.content_should_be_removed.choose 'Yes'
+      expect(form).to have_redirect_urls
+      form.redirect_urls.set 'https://example.com/redirect'
+      form.content_similar.choose 'Yes'
+      expect(form).to have_similar_urls
+      form.similar_urls.set 'https://example.com/similar'
+      form.save_and_continue.click
+    end
+
+    expect(audit_content_item)
+      .to have_success_message(text: 'Audit saved — no items remaining.')
+
+    audit_content_item.audit_form do |form|
+      expect(form.title).to have_checked_field('No')
+      expect(form.summary).to have_checked_field('Yes')
+      expect(form.page_detail).to have_checked_field('No')
+      expect(form.attachments).to have_checked_field('Yes')
+      expect(form.content_type).to have_checked_field('No')
+      expect(form.content_out_of_date).to have_checked_field('Yes')
+      expect(form.content_should_be_removed).to have_checked_field('Yes')
+      expect(form).to have_redirect_urls(text: 'https://example.com/redirect')
+      expect(form.content_similar).to have_checked_field('Yes')
+      expect(form).to have_similar_urls(text: 'https://example.com/similar')
+      expect(form).to have_notes(text: 'something')
+    end
   end
 
   scenario "clicking on yes and no buttons for redundant/similar content questions", js: true do
-    visit content_item_audit_path(content_item)
+    audit_content_item = ContentAuditTool.new.audit_content_item
+    audit_content_item.load(content_id: content_item.content_id)
 
-    expect(page).to have_no_content("Where should users be redirected to? (optional)")
-    expect(page).to have_no_content("URLs of similar pages")
+    audit_content_item.audit_form do |form|
+      expect(form).to have_no_redirect_urls
+      expect(form).to have_no_similar_urls
 
-    answer_question "Should the content be removed?", "Yes"
-    expect(page).to have_content("Where should users be redirected to? (optional)")
+      form.content_should_be_removed.choose 'Yes'
+      expect(form).to have_redirect_urls
 
-    answer_question "Should the content be removed?", "No"
-    expect(page).to have_no_content("Where should users be redirected to? (optional)")
+      form.content_should_be_removed.choose 'No'
+      expect(form).to have_no_redirect_urls
 
-    answer_question "Is this content very similar to other pages?", "Yes"
-    expect(page).to have_content("URLs of similar pages")
+      form.content_similar.choose 'Yes'
+      expect(form).to have_similar_urls
 
-    answer_question "Is this content very similar to other pages?", "No"
-    expect(page).to have_no_content("URLs of similar pages")
+      form.content_similar.choose 'No'
+      expect(form).to have_no_similar_urls
+    end
   end
 
   scenario "filling in and saving questions for redundant content", js: true do
-    visit content_item_audit_path(content_item)
-
-    answer_question "Title", "No"
-    answer_question "Summary", "Yes"
-    answer_question "Page detail", "No"
-    fill_in "Notes", with: "something"
-    answer_question "Attachments", "Yes"
-    answer_question "Content type", "No"
-    answer_question "Is the content out of date?", "Yes"
-    answer_question "Should the content be removed?", "Yes"
-    fill_in "Where should users be redirected to? (optional)", with: "http://www.example.com"
-    answer_question "Is this content very similar to other pages?", "Yes"
-    fill_in "URLs of similar pages", with: "http://www.example.com"
-
-    click_on "Save"
-
-    expect(page).to have_content("Where should users be redirected to? (optional)")
-    expect(find_field("Where should users be redirected to? (optional)").value).to eq("http://www.example.com")
-    expect(page).to have_content("URLs of similar pages")
-    expect(find_field("URLs of similar pages").value).to eq("http://www.example.com")
-
-    answer_question "Should the content be removed?", "No"
-    answer_question "Is this content very similar to other pages?", "No"
-    click_on "Save"
-
-    expect(page).to have_no_content('Where should users be redirected to? (optional)')
-    expect(page).to have_no_content("URLs of similar pages")
-
-    answer_question "Should the content be removed?", "Yes"
-    answer_question "Is this content very similar to other pages?", "Yes"
-    expect(find_field("Where should users be redirected to? (optional)").value).to eq("")
-    expect(find_field("URLs of similar pages").value).to eq("")
+    audit_content_item = ContentAuditTool.new.audit_content_item
+    audit_content_item.load(content_id: content_item.content_id)
+
+    audit_content_item.audit_form do |form|
+      form.title.choose 'No'
+      form.summary.choose 'Yes'
+      form.page_detail.choose 'No'
+      form.notes.set 'something'
+      form.attachments.choose 'Yes'
+      form.content_type.choose 'No'
+      form.content_out_of_date.choose 'Yes'
+      form.content_should_be_removed.choose 'Yes'
+      form.redirect_urls.set 'https://example.com/redirect'
+      form.content_similar.choose 'Yes'
+      form.similar_urls.set 'https://example.com/similar'
+
+      form.save_and_continue.click
+
+      form.wait_for_redirect_urls
+      form.wait_for_similar_urls
+
+      expect(form).to have_redirect_urls(text: 'https://example.com/redirect')
+      expect(form).to have_similar_urls(text: 'https://example.com/similar')
+
+      form.content_should_be_removed.choose 'No'
+      form.content_similar.choose 'No'
+
+      form.save_and_continue.click
+
+      form.wait_for_redirect_urls
+      form.wait_for_similar_urls
+
+      expect(form).to have_no_redirect_urls
+      expect(form).to have_no_similar_urls
+
+      form.content_should_be_removed.choose 'Yes'
+      form.content_similar.choose 'Yes'
+
+      expect(form).to have_redirect_urls(text: '')
+      expect(form).to have_similar_urls(text: '')
+    end
   end
 
   context "a content item is assigned to me" do
-    let!(:sabriel) do
-      create(
-        :content_item,
-        allocated_to: me,
-      )
-    end
+    let(:sabriel) { create(:content_item, allocated_to: me) }
 
     scenario "my name and organisation are shown on the content item" do
-      visit content_item_audit_path(sabriel)
+      audit_content_item = ContentAuditTool.new.audit_content_item
+      audit_content_item.load(content_id: sabriel.content_id)
 
-      expect(page).to have_content("Garth Nix")
-      expect(page).to have_content("YA Authors")
+      expect(audit_content_item.metadata)
+        .to have_assigned_to(text: 'Garth Nix YA Authors')
     end
   end
 end
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index da74bd1fd..3781c9486 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -18,6 +18,7 @@
 require "gds_api/test_helpers/publishing_api_v2"
 require "pry"
 require "database_cleaner"
+require "site_prism"
 
 RSpec.configure do |config|
   config.disable_monkey_patching!
@@ -54,3 +55,5 @@ def use_truncation?
     DatabaseCleaner.clean
   end
 end
+
+Dir['./spec/support/**/*.rb'].sort.each { |f| require f }
diff --git a/spec/support/pages/audit/content_audit_tool.rb b/spec/support/pages/audit/content_audit_tool.rb
new file mode 100644
index 000000000..1adc1c008
--- /dev/null
+++ b/spec/support/pages/audit/content_audit_tool.rb
@@ -0,0 +1,7 @@
+require_relative 'content_audit_tool/audit_content_item_page'
+
+class ContentAuditTool
+  def audit_content_item
+    AuditContentItemPage.new
+  end
+end
diff --git a/spec/support/pages/audit/content_audit_tool/audit_content_item_page.rb b/spec/support/pages/audit/content_audit_tool/audit_content_item_page.rb
new file mode 100644
index 000000000..f2266dd52
--- /dev/null
+++ b/spec/support/pages/audit/content_audit_tool/audit_content_item_page.rb
@@ -0,0 +1,31 @@
+require 'site_prism/page'
+
+class AuditContentItemPage < SitePrism::Page
+  set_url '/content_items{/content_id}/audit{?query*}'
+
+  element :error_message, '.alert-danger'
+  element :success_message, '.alert-success'
+
+  element :content_item_description, 'p.description'
+  element :content_item_title, 'h2'
+  element :questions_title, 'h4'
+
+  section :audit_form, 'form' do
+    element :attachments, '.change-attachments'
+    element :content_out_of_date, '.outdated'
+    element :content_should_be_removed, '.redundant'
+    element :content_similar, '.similar'
+    element :content_type, '.reformat'
+    element :notes, '.notes textarea'
+    element :page_detail, '.change-body'
+    element :redirect_urls, '#audits_audit_redirect_urls'
+    element :save_and_continue, 'input[type=submit]'
+    element :similar_urls, '#audits_audit_similar_urls'
+    element :summary, '.change-description'
+    element :title, '.change-title'
+  end
+
+  section :metadata, '#metadata' do
+    element :assigned_to, '#allocated'
+  end
+end

From ccc4052653c30f42794cf5fd1f17bfeebc133666 Mon Sep 17 00:00:00 2001
From: Andrew Garner <andrew@andrewgarner.com>
Date: Mon, 16 Oct 2017 18:05:26 +0100
Subject: [PATCH 2/4] Refactor audit feature test to improve readability

This changes the style the test are written in to add clear separation
between the Arrange, Act, Assert parts of the test.

See https://about.futurelearn.com/blog/how-we-write-readable-feature-tests-with-rspec
---
 spec/features/audit/audit/audit_spec.rb | 198 +++++++++++++++---------
 1 file changed, 121 insertions(+), 77 deletions(-)

diff --git a/spec/features/audit/audit/audit_spec.rb b/spec/features/audit/audit/audit_spec.rb
index 1907fe092..1b8cb3ccc 100644
--- a/spec/features/audit/audit/audit_spec.rb
+++ b/spec/features/audit/audit/audit_spec.rb
@@ -1,56 +1,86 @@
-RSpec.feature "Auditing a content item", type: :feature do
-  let!(:content_item) do
-    create(
-      :content_item,
-      title: "Flooding",
-      description: "All about flooding.",
-      base_path: "/flooding",
-      publishing_app: "whitehall",
-    )
+RSpec.feature 'Auditing a content item', type: :feature do
+  scenario 'information about the published content is available' do
+    given_i_am_auditing_a_content_item
+    then_the_title_is_shown_linking_to_the_published_content
+    and_the_description_is_shown
   end
 
-  let!(:my_organisation) do
-    create(
-      :organisation,
-      title: "YA Authors",
-    )
+  scenario 'information about the assignee is available' do
+    given_i_am_auditing_a_content_item
+    then_the_name_of_the_assignee_is_shown
   end
 
-  let!(:me) do
-    create(
-      :user,
-      name: "Garth Nix",
-      organisation: my_organisation,
-    )
+  scenario 'all questions must be answered' do
+    given_i_am_auditing_a_content_item
+    then_i_am_prompted_to_consider_if_it_needs_to_change
+    when_i_answer_some_of_the_questions
+    then_an_error_message_is_shown
   end
 
-  scenario "auditing a content item" do
-    audit_content_item = ContentAuditTool.new.audit_content_item
-    audit_content_item.load(content_id: content_item.content_id)
+  scenario 'answers are remembered' do
+    given_i_am_auditing_a_content_item
+    when_i_answer_all_of_the_questions
+    then_a_success_message_is_shown
+    and_my_answers_to_the_questions_are_remembered
+  end
 
-    expect(page).to_not have_selector(".nav")
+  scenario 'clicking on yes and no buttons for redundant/similar content questions', js: true do
+    given_i_am_auditing_a_content_item
+    then_i_am_prompted_to_specify_redirect_urls_if_the_content_should_be_removed
+  end
 
-    expect(audit_content_item.content_item_title)
-      .to have_link('Flooding', href: 'https://gov.uk/flooding')
+  scenario 'filling in and saving questions for redundant content', js: true do
+    given_i_am_auditing_a_content_item
+    when_i_specify_urls
+    then_the_urls_i_specified_are_shown
+    when_i_specify_that_the_content_is_relevant
+    then_the_urls_i_previously_specified_are_discarded
+  end
 
-    expect(audit_content_item)
-      .to have_content_item_description(text: 'All about flooding.')
+private
+
+  def given_i_am_auditing_a_content_item
+    organisation = create(:organisation, title: 'YA Authors')
+    user = create(:user, name: 'Garth Nix', organisation: organisation)
+
+    content_item = create(
+      :content_item,
+      allocated_to: user,
+      title: 'Flooding',
+      description: 'All about flooding.',
+      base_path: '/flooding',
+      publishing_app: 'whitehall',
+    )
+
+    @audit_content_item = ContentAuditTool.new.audit_content_item
+    @audit_content_item.load(content_id: content_item.content_id)
+  end
 
-    expect(audit_content_item)
+  def then_i_am_prompted_to_consider_if_it_needs_to_change
+    expect(@audit_content_item)
       .to have_questions_title(text: 'Do these things need to change?')
+  end
 
-    audit_content_item.audit_form do |form|
+  def then_the_name_of_the_assignee_is_shown
+    expect(@audit_content_item.metadata)
+      .to have_assigned_to(text: 'Garth Nix YA Authors')
+  end
+
+  def when_i_answer_some_of_the_questions
+    @audit_content_item.audit_form do |form|
       form.title.choose 'No'
       form.summary.choose 'Yes'
       form.page_detail.choose 'No'
       form.notes.set 'something'
       form.save_and_continue.click
     end
+  end
 
-    expect(audit_content_item)
-      .to have_error_message(text: 'Please answer all the questions.')
-
-    audit_content_item.audit_form do |form|
+  def when_i_answer_all_of_the_questions
+    @audit_content_item.audit_form do |form|
+      form.title.choose 'No'
+      form.summary.choose 'Yes'
+      form.page_detail.choose 'No'
       form.attachments.choose 'Yes'
       form.content_type.choose 'No'
       form.content_out_of_date.choose 'Yes'
@@ -60,13 +90,60 @@
       form.content_similar.choose 'Yes'
       expect(form).to have_similar_urls
       form.similar_urls.set 'https://example.com/similar'
+      form.notes.set 'something'
       form.save_and_continue.click
     end
+  end
+
+  def when_i_specify_that_the_content_is_relevant
+    @audit_content_item.audit_form do |form|
+      form.content_should_be_removed.choose 'No'
+      form.content_similar.choose 'No'
+
+      form.save_and_continue.click
+    end
+  end
 
-    expect(audit_content_item)
+  def when_i_specify_urls
+    @audit_content_item.audit_form do |form|
+      form.title.choose 'No'
+      form.summary.choose 'Yes'
+      form.page_detail.choose 'No'
+      form.notes.set 'something'
+      form.attachments.choose 'Yes'
+      form.content_type.choose 'No'
+      form.content_out_of_date.choose 'Yes'
+      form.content_should_be_removed.choose 'Yes'
+      form.redirect_urls.set 'https://example.com/redirect'
+      form.content_similar.choose 'Yes'
+      form.similar_urls.set 'https://example.com/similar'
+
+      form.save_and_continue.click
+    end
+  end
+
+  def then_a_success_message_is_shown
+    expect(@audit_content_item)
       .to have_success_message(text: 'Audit saved — no items remaining.')
+  end
+
+  def then_an_error_message_is_shown
+    expect(@audit_content_item)
+      .to have_error_message(text: 'Please answer all the questions.')
+  end
+
+  def then_the_title_is_shown_linking_to_the_published_content
+    expect(@audit_content_item.content_item_title)
+      .to have_link('Flooding', href: 'https://gov.uk/flooding')
+  end
+
+  def and_the_description_is_shown
+    expect(@audit_content_item)
+      .to have_content_item_description(text: 'All about flooding.')
+  end
 
-    audit_content_item.audit_form do |form|
+  def and_my_answers_to_the_questions_are_remembered
+    @audit_content_item.audit_form do |form|
       expect(form.title).to have_checked_field('No')
       expect(form.summary).to have_checked_field('Yes')
       expect(form.page_detail).to have_checked_field('No')
@@ -81,11 +158,8 @@
     end
   end
 
-  scenario "clicking on yes and no buttons for redundant/similar content questions", js: true do
-    audit_content_item = ContentAuditTool.new.audit_content_item
-    audit_content_item.load(content_id: content_item.content_id)
-
-    audit_content_item.audit_form do |form|
+  def then_i_am_prompted_to_specify_redirect_urls_if_the_content_should_be_removed
+    @audit_content_item.audit_form do |form|
       expect(form).to have_no_redirect_urls
       expect(form).to have_no_similar_urls
 
@@ -103,36 +177,18 @@
     end
   end
 
-  scenario "filling in and saving questions for redundant content", js: true do
-    audit_content_item = ContentAuditTool.new.audit_content_item
-    audit_content_item.load(content_id: content_item.content_id)
-
-    audit_content_item.audit_form do |form|
-      form.title.choose 'No'
-      form.summary.choose 'Yes'
-      form.page_detail.choose 'No'
-      form.notes.set 'something'
-      form.attachments.choose 'Yes'
-      form.content_type.choose 'No'
-      form.content_out_of_date.choose 'Yes'
-      form.content_should_be_removed.choose 'Yes'
-      form.redirect_urls.set 'https://example.com/redirect'
-      form.content_similar.choose 'Yes'
-      form.similar_urls.set 'https://example.com/similar'
-
-      form.save_and_continue.click
-
+  def then_the_urls_i_specified_are_shown
+    @audit_content_item.audit_form do |form|
       form.wait_for_redirect_urls
       form.wait_for_similar_urls
 
       expect(form).to have_redirect_urls(text: 'https://example.com/redirect')
       expect(form).to have_similar_urls(text: 'https://example.com/similar')
+    end
+  end
 
-      form.content_should_be_removed.choose 'No'
-      form.content_similar.choose 'No'
-
-      form.save_and_continue.click
-
+  def then_the_urls_i_previously_specified_are_discarded
+    @audit_content_item.audit_form do |form|
       form.wait_for_redirect_urls
       form.wait_for_similar_urls
 
@@ -146,16 +202,4 @@
       expect(form).to have_similar_urls(text: '')
     end
   end
-
-  context "a content item is assigned to me" do
-    let(:sabriel) { create(:content_item, allocated_to: me) }
-
-    scenario "my name and organisation are shown on the content item" do
-      audit_content_item = ContentAuditTool.new.audit_content_item
-      audit_content_item.load(content_id: sabriel.content_id)
-
-      expect(audit_content_item.metadata)
-        .to have_assigned_to(text: 'Garth Nix YA Authors')
-    end
-  end
 end

From bdb80a381e53e100322a71a62c71f1f7ce3e7068 Mon Sep 17 00:00:00 2001
From: Andrew Garner <andrew@andrewgarner.com>
Date: Tue, 17 Oct 2017 09:50:09 +0100
Subject: [PATCH 3/4] Use a Page Object for audit metadata feature spec

---
 spec/features/audit/audit/metadata_spec.rb    | 74 ++++++++++---------
 .../audit_content_item_page.rb                |  9 +++
 2 files changed, 50 insertions(+), 33 deletions(-)

diff --git a/spec/features/audit/audit/metadata_spec.rb b/spec/features/audit/audit/metadata_spec.rb
index a56de4e67..79c051156 100644
--- a/spec/features/audit/audit/metadata_spec.rb
+++ b/spec/features/audit/audit/metadata_spec.rb
@@ -27,20 +27,21 @@
   end
 
   scenario "showing minimal metadata next to the audit quesionnaire" do
-    visit content_item_audit_path(content_item)
-
-    within("#metadata") do
-      expect(page).to have_selector("#allocated", text: "Assigned to No one")
-      expect(page).to have_selector("#audited", text: "Not audited yet")
-      expect(page).to have_selector("#organisations", text: "None")
-      expect(page).to have_selector("#last-updated", text: "Never")
-      expect(page).to have_selector("#content-type", text: "Document Collection")
-      expect(page).to have_selector("#guidance", text: "No")
-      expect(page).to have_selector("#topics", text: "None")
-      expect(page).to have_selector("#policy-areas", text: "None")
-      expect(page).to have_selector("#withdrawn", text: "No")
-      expect(page).to have_selector("#pageviews", text: "0 in the last month")
-      expect(page).to have_selector("#pageviews", text: "0 in the last six months")
+    @audit_content_item = ContentAuditTool.new.audit_content_item
+    @audit_content_item.load(content_id: content_item.content_id)
+
+    @audit_content_item.metadata do |metadata|
+      expect(metadata).to have_assigned_to(text: 'No one')
+      expect(metadata).to have_audited(text: 'Not audited yet')
+      expect(metadata).to have_content_type(text: 'Document Collection')
+      expect(metadata).to have_guidance(text: 'No')
+      expect(metadata).to have_last_major_update(text: 'Never')
+      expect(metadata).to have_organisations(text: 'None')
+      expect(metadata).to have_policy_areas(text: 'None')
+      expect(metadata).to have_topics(text: 'None')
+      expect(metadata).to have_unique_page_views(text: '0 in the last month')
+      expect(metadata).to have_unique_page_views(text: '0 in the last six months')
+      expect(metadata).to have_withdrawn(text: 'No')
     end
   end
 
@@ -71,25 +72,32 @@ def create_linked_content(link_type, title)
     create_linked_content("policy_areas", "Borders and Immigration")
 
     cbbc = create(:organisation, title: "CBBC")
-    edd = create(:user, name: "Edd the Duck", organisation: cbbc)
-    create(:allocation, content_item: content_item, user: edd)
-
-    visit content_item_audit_path(content_item)
-
-    within("#metadata") do
-      allocated_text = "Assigned to Edd the Duck CBBC"
-      audited_text = "Audited 01/01/17 (less than a minute ago) by Harper Lee Authors"
-      expect(page).to have_selector("#allocated", text: allocated_text)
-      expect(page).to have_selector("#audited", text: audited_text)
-      expect(page).to have_selector("#organisations", text: "Organisations Home office")
-      expect(page).to have_selector("#last-updated", text: "03/01/17 (2 days ago)")
-      expect(page).to have_selector("#content-type", text: "Guidance")
-      expect(page).to have_selector("#guidance", text: "Yes")
-      expect(page).to have_selector("#topics", text: "Borders, Immigration")
-      expect(page).to have_selector("#policy-areas", text: "Borders and Immigration")
-      expect(page).to have_selector("#withdrawn", text: "No")
-      expect(page).to have_selector("#pageviews", text: "1,234 in the last month")
-      expect(page).to have_selector("#pageviews", text: "12,345 in the last six months")
+    edd_the_duck = create(:user, name: "Edd the Duck", organisation: cbbc)
+
+    create(:allocation, content_item: content_item, user: edd_the_duck)
+
+    @audit_content_item = ContentAuditTool.new.audit_content_item
+    @audit_content_item.load(content_id: content_item.content_id)
+
+    @audit_content_item.metadata do |metadata|
+      expect(metadata).to have_assigned_to(
+                            text: 'Edd the Duck CBBC',
+                          )
+
+      expect(metadata).to have_audited(
+                            text: '01/01/17 (less than a minute ago) ' \
+                                  'by Harper Lee Authors',
+                          )
+
+      expect(metadata).to have_organisations(text: 'Home office')
+      expect(metadata).to have_last_major_update(text: '03/01/17 (2 days ago)')
+      expect(metadata).to have_content_type(text: 'Guidance')
+      expect(metadata).to have_guidance(text: 'Yes')
+      expect(metadata).to have_topics(text: 'Borders, Immigration')
+      expect(metadata).to have_policy_areas(text: 'Borders and Immigration')
+      expect(metadata).to have_withdrawn(text: 'No')
+      expect(metadata).to have_unique_page_views(text: '1,234 in the last month')
+      expect(metadata).to have_unique_page_views(text: '12,345 in the last six months')
     end
   end
 end
diff --git a/spec/support/pages/audit/content_audit_tool/audit_content_item_page.rb b/spec/support/pages/audit/content_audit_tool/audit_content_item_page.rb
index f2266dd52..30b81d365 100644
--- a/spec/support/pages/audit/content_audit_tool/audit_content_item_page.rb
+++ b/spec/support/pages/audit/content_audit_tool/audit_content_item_page.rb
@@ -27,5 +27,14 @@ class AuditContentItemPage < SitePrism::Page
 
   section :metadata, '#metadata' do
     element :assigned_to, '#allocated'
+    element :audited, '#audited'
+    element :content_type, '#content-type'
+    element :guidance, '#guidance'
+    element :last_major_update, '#last-updated'
+    element :organisations, '#organisations'
+    element :policy_areas, '#policy-areas'
+    element :topics, '#topics'
+    element :unique_page_views, '#pageviews'
+    element :withdrawn, '#withdrawn'
   end
 end

From 118b6149c46728e72fed080a300f783ddc7fe829 Mon Sep 17 00:00:00 2001
From: Andrew Garner <andrew@andrewgarner.com>
Date: Tue, 17 Oct 2017 14:57:14 +0100
Subject: [PATCH 4/4] Refactor audit metadata feature test to improve
 readability

---
 spec/features/audit/audit/metadata_spec.rb | 235 ++++++++++++++-------
 1 file changed, 162 insertions(+), 73 deletions(-)

diff --git a/spec/features/audit/audit/metadata_spec.rb b/spec/features/audit/audit/metadata_spec.rb
index 79c051156..2bf27739a 100644
--- a/spec/features/audit/audit/metadata_spec.rb
+++ b/spec/features/audit/audit/metadata_spec.rb
@@ -1,103 +1,192 @@
-RSpec.feature "Audit metadata", type: :feature do
-  let!(:my_organisation) do
+RSpec.feature 'Audit metadata', type: :feature do
+  scenario 'showing minimal metadata next to the audit questionnaire' do
+    given_a_content_item_that_has_not_been_audited_or_assigned
+    when_i_audit_the_content_item
+    then_i_am_shown_that_the_content_item_is_assigned_to_no_one
+    and_i_am_shown_that_the_content_item_is_not_audited_yet
+    and_i_am_shown_the_content_type
+    and_i_am_shown_that_the_content_is_guidance
+    and_i_am_shown_that_the_content_has_not_had_a_major_update
+    and_i_am_shown_that_the_content_has_no_organisations
+    and_i_am_shown_that_the_content_has_no_policy_areas
+    and_i_am_shown_that_the_content_has_not_been_withdrawn
+    and_i_am_shown_that_the_content_has_no_topics
+    and_i_am_shown_that_the_content_has_no_unique_page_views
+  end
+
+  scenario 'showing maximal metadata next to the audit questionnaire' do
+    given_a_content_item_that_has_been_tagged_and_audited_and_assigned
+    when_i_audit_the_content_item
+    then_i_am_shown_that_the_content_item_has_been_assigned
+    and_i_am_shown_that_the_content_item_has_been_audited
+    and_i_am_shown_the_organisations_the_content_item_is_tagged_to
+    and_i_am_shown_when_the_last_major_update_happened
+    and_i_am_shown_the_content_type
+    and_i_am_shown_that_the_content_is_guidance
+    and_i_am_shown_the_topics_the_content_item_is_tagged_to
+    and_i_am_shown_the_policy_areas_the_content_item_is_tagged_to
+    and_i_am_shown_that_the_content_has_not_been_withdrawn
+    and_i_am_shown_the_unique_page_views
+  end
+
+  around do |example|
+    Timecop.freeze(2017, 1, 1) { example.run }
+  end
+
+  def given_a_content_item_that_has_been_tagged_and_audited_and_assigned
+    auditor = create(
+      :user,
+      name: 'Harper Lee',
+      organisation: create(:organisation, title: 'Authors'),
+    )
+
+    assignee = create(
+      :user,
+      name: 'Edd The Duck',
+      organisation: create(:organisation, title: 'CBBC'),
+    )
+
+    @content_item = create(
+      :content_item,
+      document_type: 'guidance',
+      one_month_page_views: 1234,
+      six_months_page_views: 12345,
+      public_updated_at: '2017-01-03',
+    )
+
     create(
-      :organisation,
-      title: "Authors",
+      :link,
+      link_type: :organisations,
+      source: @content_item,
+      target: create(:content_item, title: 'Home office'),
     )
-  end
 
-  let!(:me) do
     create(
-      :user,
-      name: "Harper Lee",
-      organisation: my_organisation,
+      :link,
+      link_type: :topics,
+      source: @content_item,
+      target: create(:content_item, title: 'Borders'),
+    )
+
+    create(
+      :link,
+      link_type: :topics,
+      source: @content_item,
+      target: create(:content_item, title: 'Immigration'),
     )
-  end
 
-  let!(:content_item) do
     create(
+      :link,
+      link_type: :policy_areas,
+      source: @content_item,
+      target: create(:content_item, title: 'Borders and Immigration'),
+    )
+
+    create(:allocation, content_item: @content_item, user: assignee)
+    create(:audit, content_item: @content_item, user: auditor)
+  end
+
+  def given_a_content_item_that_has_not_been_audited_or_assigned
+    create(:user)
+
+    @content_item = create(
       :content_item,
+      document_type: 'guidance',
       public_updated_at: nil,
-      document_type: "document_collection",
     )
   end
 
-  around do |example|
-    Timecop.freeze(2017, 1, 1) { example.run }
+  def when_i_audit_the_content_item
+    @audit_content_item = ContentAuditTool.new.audit_content_item
+    @audit_content_item.load(content_id: @content_item.content_id)
   end
 
-  scenario "showing minimal metadata next to the audit quesionnaire" do
-    @audit_content_item = ContentAuditTool.new.audit_content_item
-    @audit_content_item.load(content_id: content_item.content_id)
+  def then_i_am_shown_that_the_content_item_is_assigned_to_no_one
+    expect(@audit_content_item.metadata).to have_assigned_to(text: 'No one')
+  end
 
-    @audit_content_item.metadata do |metadata|
-      expect(metadata).to have_assigned_to(text: 'No one')
-      expect(metadata).to have_audited(text: 'Not audited yet')
-      expect(metadata).to have_content_type(text: 'Document Collection')
-      expect(metadata).to have_guidance(text: 'No')
-      expect(metadata).to have_last_major_update(text: 'Never')
-      expect(metadata).to have_organisations(text: 'None')
-      expect(metadata).to have_policy_areas(text: 'None')
-      expect(metadata).to have_topics(text: 'None')
-      expect(metadata).to have_unique_page_views(text: '0 in the last month')
-      expect(metadata).to have_unique_page_views(text: '0 in the last six months')
-      expect(metadata).to have_withdrawn(text: 'No')
-    end
+  def then_i_am_shown_that_the_content_item_has_been_assigned
+    expect(@audit_content_item.metadata)
+      .to have_assigned_to(text: 'Edd The Duck CBBC')
   end
 
-  def create_linked_content(link_type, title)
-    target = create(:content_item, title: title)
+  def and_i_am_shown_that_the_content_item_is_not_audited_yet
+    expect(@audit_content_item.metadata)
+      .to have_audited(text: 'Not audited yet')
+  end
 
-    create(
-      :link,
-      link_type: link_type,
-      source_content_id: content_item.content_id,
-      target_content_id: target.content_id,
-    )
+  def and_i_am_shown_that_the_content_item_has_been_audited
+    expect(@audit_content_item.metadata)
+      .to have_audited(text: '01/01/17 (less than a minute ago) ' \
+                             'by Harper Lee Authors')
   end
 
-  scenario "showing maximal metadata next to the audit questionnaire" do
-    content_item.update!(
-      public_updated_at: "2017-01-03",
-      one_month_page_views: 1234,
-      six_months_page_views: 12345,
-      document_type: "guidance",
-    )
+  def and_i_am_shown_the_content_type
+    expect(@audit_content_item.metadata)
+      .to have_content_type(text: 'Guidance')
+  end
 
-    create(:audit, content_item: content_item, user: me)
+  def and_i_am_shown_that_the_content_has_not_had_a_major_update
+    expect(@audit_content_item.metadata).to have_last_major_update(text: 'Never')
+  end
 
-    create_linked_content("organisations", "Home office")
-    create_linked_content("topics", "Immigration")
-    create_linked_content("topics", "Borders")
-    create_linked_content("policy_areas", "Borders and Immigration")
+  def and_i_am_shown_that_the_content_has_no_organisations
+    expect(@audit_content_item.metadata).to have_organisations(text: 'None')
+  end
 
-    cbbc = create(:organisation, title: "CBBC")
-    edd_the_duck = create(:user, name: "Edd the Duck", organisation: cbbc)
+  def and_i_am_shown_that_the_content_has_no_policy_areas
+    expect(@audit_content_item.metadata).to have_policy_areas(text: 'None')
+  end
 
-    create(:allocation, content_item: content_item, user: edd_the_duck)
+  def and_i_am_shown_that_the_content_has_no_topics
+    expect(@audit_content_item.metadata).to have_topics(text: 'None')
+  end
 
-    @audit_content_item = ContentAuditTool.new.audit_content_item
-    @audit_content_item.load(content_id: content_item.content_id)
+  def and_i_am_shown_that_the_content_has_no_unique_page_views
+    @audit_content_item.metadata do |metadata|
+      expect(metadata)
+        .to have_unique_page_views(text: '0 in the last month')
 
+      expect(metadata)
+        .to have_unique_page_views(text: '0 in the last six months')
+    end
+  end
+
+  def and_i_am_shown_that_the_content_has_not_been_withdrawn
+    expect(@audit_content_item.metadata).to have_withdrawn(text: 'No')
+  end
+
+  def and_i_am_shown_the_organisations_the_content_item_is_tagged_to
+    expect(@audit_content_item.metadata)
+      .to have_organisations(text: 'Home office')
+  end
+
+  def and_i_am_shown_when_the_last_major_update_happened
+    expect(@audit_content_item.metadata)
+      .to have_last_major_update(text: '03/01/17 (2 days ago)')
+  end
+
+  def and_i_am_shown_that_the_content_is_guidance
+    expect(@audit_content_item.metadata).to have_content_type(text: 'Guidance')
+  end
+
+  def and_i_am_shown_the_topics_the_content_item_is_tagged_to
+    expect(@audit_content_item.metadata)
+      .to have_topics(text: 'Borders, Immigration')
+  end
+
+  def and_i_am_shown_the_policy_areas_the_content_item_is_tagged_to
+    expect(@audit_content_item.metadata)
+      .to have_policy_areas(text: 'Borders and Immigration')
+  end
+
+  def and_i_am_shown_the_unique_page_views
     @audit_content_item.metadata do |metadata|
-      expect(metadata).to have_assigned_to(
-                            text: 'Edd the Duck CBBC',
-                          )
-
-      expect(metadata).to have_audited(
-                            text: '01/01/17 (less than a minute ago) ' \
-                                  'by Harper Lee Authors',
-                          )
-
-      expect(metadata).to have_organisations(text: 'Home office')
-      expect(metadata).to have_last_major_update(text: '03/01/17 (2 days ago)')
-      expect(metadata).to have_content_type(text: 'Guidance')
-      expect(metadata).to have_guidance(text: 'Yes')
-      expect(metadata).to have_topics(text: 'Borders, Immigration')
-      expect(metadata).to have_policy_areas(text: 'Borders and Immigration')
-      expect(metadata).to have_withdrawn(text: 'No')
-      expect(metadata).to have_unique_page_views(text: '1,234 in the last month')
-      expect(metadata).to have_unique_page_views(text: '12,345 in the last six months')
+      expect(metadata)
+        .to have_unique_page_views(text: '1,234 in the last month')
+
+      expect(metadata)
+        .to have_unique_page_views(text: '12,345 in the last six months')
     end
   end
 end